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

The `field.plotIndexes` array can grow large too, leading to DOS

joshuajee

Relevant Github Links

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/LibDibbler.sol#L100
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/LibDibbler.sol#L412
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/LibDibbler.sol#L394
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/market/MarketplaceFacet/PodTransfer.sol#L79
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/market/MarketplaceFacet/PodTransfer.sol#L47

Summary

There is no limit to the size of the plot that an account can hold this will lead to DOS as the field.plotIndexes array grows too large.

Vulnerability Details

When sowing bean, the index is added to s.accts[account].fields[s.sys.activeField].plotIndexes.push(index) this array can grow large and cause a DOS in the system.

function sow(
uint256 beans,
uint256 _morningTemperature,
address account,
bool abovePeg
) internal returns (uint256) {
...
s.accts[account].fields[s.sys.activeField].plots[index] = pods;
@-> s.accts[account].fields[s.sys.activeField].plotIndexes.push(index);
}

This array can also grow when the plot is transferred.

function _transferPlot(
address from,
address to,
uint256 fieldId,
uint256 index,
uint256 start,
uint256 amount
) internal {
require(from != to, "Field: Cannot transfer Pods to oneself.");
@-> insertPlot(to, fieldId, index + start, amount);
removePlot(from, fieldId, index, start, amount + start);
emit PlotTransfer(from, to, index + start, amount);
}
​
function insertPlot(address account, uint256 fieldId, uint256 index, uint256 amount) internal {
s.accts[account].fields[fieldId].plots[index] = amount;
@-> s.accts[account].fields[fieldId].plotIndexes.push(index);
}

The function below loops through the field.plotIndexes and will get more expensive as the array grows larger. The findPlotIndexForAccount is used in the LibDribbler.removePlotIndexFromAccount function that is used extensively in the system including pod transfer.

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;
}

Impact

  1. Plot cannot be removed from an account because it relies on the findPlotIndexForAccount.

  2. Plot transfer will be broken because it relies on the LibDribbler.removePlotIndexFromAccount.

  3. Pod market will be broken.

  4. Plot cannot be harvested

Tools Used

Manual Analysis

Recommendations

Set a maximum value for the length of the s.accts[account].fields[s.sys.activeField].plotIndexes array.

Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational/Gas

Invalid as per docs https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity

Appeal created

joshuajee Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

`plotIndexes` array can be spammed by 1 wei transfers

inallhonesty Lead Judge 11 months 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.