DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: medium
Valid

Incorrect Loop Variable Increment in ReseedField Contract

Summary

The ReseedField contract has an issue in its init function where the wrong loop variable is being incremented in a nested loop. This error can lead to an infinite loop or out-of-bounds access, potentially causing the function to revert or consume all available gas.

Vulnerability Details

In the init function of the ReseedField contract, there is a nested loop that iterates over accountPlots and their respective plots. The inner loop incorrectly increments the outer loop variable i instead of the inner loop variable j:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/4e0ad0b964f74a1b4880114f4dd5b339bc69cd3e/protocol/contracts/beanstalk/init/reseed/L2/ReseedField.sol#L49

for (uint i; i < accountPlots.length; i++) {
for (uint j; j < accountPlots[i].plots.length; i++) {
}
}

This mistake causes the inner loop to never terminate properly, as j is not incremented. Depending on the input data, this could lead to an infinite loop or an out-of-bounds array access.

Impact

The impact of this vulnerability is severe:

  1. Gas Exhaustion: The function may consume all available gas, causing the transaction to fail.

  2. Denial of Service: The contract's init function becomes unusable, preventing the proper initialization of the field.

  3. Incorrect State: If the function doesn't revert due to gas exhaustion, it may partially update the contract state, leaving it in an inconsistent or incorrect state.

This bug effectively renders the init function non-functional, which is critical as it's responsible for re-initializing the entire field state.

Tools Used

manual code review.

Recommendations

The inner loop should increment j instead of i as shown below:

function init(
MigratedPlotData[] calldata accountPlots,
uint256 totalPods,
uint256 harvestable,
uint256 harvested,
uint256 fieldId,
uint8 initialTemperature
) external {
uint256 calculatedTotalPods;
for (uint i; i < accountPlots.length; i++) {
- for (uint j; j < accountPlots[i].plots.length; i++) {
+ for (uint j; j < accountPlots[i].plots.length; j++) {
uint256 podIndex = accountPlots[i].plots[j].podIndex;
uint256 podAmount = accountPlots[i].plots[j].podAmounts;
s.accts[accountPlots[i].account].fields[fieldId].plots[podIndex] = podAmount;
s.accts[accountPlots[i].account].fields[fieldId].plotIndexes.push(podIndex);
emit MigratedPlot(accountPlots[i].account, podIndex, podAmount);
calculatedTotalPods += podAmount;
}
}
}
Updates

Lead Judging Commences

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

Very broken loop in ReseedField::init

Appeal created

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

Very broken loop in ReseedField::init

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

Very broken loop in ReseedField::init

Support

FAQs

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