Beanstalk Part 1
Beanstalk
DeFiHardhatOracleProxyUpdates
100,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Incorrect loop termination condition in `rewardToFertilizer`

0xbeastboy

Summary

In Silo:rewardToFertilizer there is a condition newTotalBpf >= firstEndBpf in the while loop. This condition implies that the loop continues as long as the total new beans per fertilizer is greater than or equal to the end beans per fertilizer of the first fertilizer to run out. At the end of the loop firstEndBpf has assigned s.fFirst value, which is never updated or changed within the loop. Same value is assigned again and again.

Vulnerability Details

See the following code:

function rewardToFertilizer(uint256 amount)
internal
returns (uint256 newFertilized)
{
// 1/3 of new Beans being minted
uint256 maxNewFertilized = amount.div(FERTILIZER_DENOMINATOR);
​
// Get the new Beans per Fertilizer and the total new Beans per Fertilizer
uint256 newBpf = maxNewFertilized.div(s.activeFertilizer);
uint256 oldTotalBpf = s.bpf;
uint256 newTotalBpf = oldTotalBpf.add(newBpf);
​
// Get the end Beans per Fertilizer of the first Fertilizer to run out.
uint256 firstEndBpf = s.fFirst;
​
// If the next fertilizer is going to run out, then step BPF according
while(newTotalBpf >= firstEndBpf) {
// Calculate BPF and new Fertilized when the next Fertilizer ID ends
newBpf = firstEndBpf.sub(oldTotalBpf);
newFertilized = newFertilized.add(newBpf.mul(s.activeFertilizer));
​
// If there is no more fertilizer, end
if (!LibFertilizer.pop()) {
s.bpf = uint128(firstEndBpf); // SafeCast unnecessary here.
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized);
require(s.fertilizedIndex == s.unfertilizedIndex, "Paid != owed");
return newFertilized;
}
​
// Calculate new Beans per Fertilizer values
newBpf = maxNewFertilized.sub(newFertilized).div(s.activeFertilizer);
oldTotalBpf = firstEndBpf;
newTotalBpf = oldTotalBpf.add(newBpf);
firstEndBpf = s.fFirst;
}
​
// Distribute the rest of the Fertilized Beans
s.bpf = uint128(newTotalBpf); // SafeCast unnecessary here.
newFertilized = newFertilized.add(newBpf.mul(s.activeFertilizer));
s.fertilizedIndex = s.fertilizedIndex.add(newFertilized);
}

Impact

If s.fFirst is not properly updated within the loop and remains constant or increases indefinitely, the loop condition newTotalBpf >= firstEndBpf may always evaluate to true, causing the loop to become an infinite loop. This could result in excessive gas consumption and potentially render the contract unusable.

Tools Used

Manual Review

Recommendations

To address this potential issue, you should ensure that s.fFirst is updated correctly within the loop so that the loop condition terminates as expected. Alternatively, you may need to adjust the loop condition to properly reflect the termination criteria based on the logic of your contract.

Updates

Lead Judging Commences

giovannidisiena Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Informational/Invalid

Support

FAQs

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