Beanstalk: The Finale
Beanstalk
DeFiHardhatFoundry
250,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Attacker can spam Plots to victim to cause DOS on Plot transfer

T1MOH

Summary

To remove certain Plot index from account it loops through array plotIndexes:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/LibDibbler.sol#L385-L419

function removePlotIndexFromAccount(
address account,
uint256 fieldId,
uint256 plotIndex
) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
@> uint256 i = findPlotIndexForAccount(account, fieldId, plotIndex);
Field storage field = s.accts[account].fields[fieldId];
field.plotIndexes[i] = field.plotIndexes[field.plotIndexes.length - 1];
field.plotIndexes.pop();
}
​
/**
* @notice finds the index of a plot in an accounts plotIndex list.
*/
function findPlotIndexForAccount(
address account,
uint256 fieldId,
uint256 plotIndex
) internal view returns (uint256 i) {
AppStorage storage s = LibAppStorage.diamondStorage();
Field storage field = s.accts[account].fields[fieldId];
uint256[] memory plotIndexes = field.plotIndexes;
uint256 length = plotIndexes.length;
while (plotIndexes[i] != plotIndex) {
i++;
if (i >= length) {
revert("Id not found");
}
}
return i;
}

PlotIndex is removed in several sceanrios:

  1. When User transfers his Plot, i.e. in PodTransfer.removePlot()

  2. When User harvests that PlotIndex, i.e. in FieldFacet._harvestPlot()

Attacker can just spam victim by transferring 1 wei Plots. It will populate array plotIndexes too much. As a result that removal will revert with out-of-gas error.

Impact

User can't transfer Plots because it tries to find and remove certain index. It will revert with out-of-gas error because Griefer transferred too many 1 wei plots previously.

Worth noting that Protocol will operate on L2 with cheap gas, so it's much easier now to perform attack from griefer's side.

At will of attacker he can poison anyone's account and block interaction with existing Plots.

Tools Used

Manual Review

Recommendations

Do not track array plotIndexes on-chain. Track events to get array of Plot indexes off-chain

Or you can refactor to use Enumerable set instead of array.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`plotIndexes` array can be spammed by 1 wei transfers

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.