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

`LibSilo.transferStalk()` uses incorrect formula to roundUp

T1MOH

Summary

It calculates proportional amount of Roots to transfer when transfers Stalk to another account. It wants to roundUp the division, however it's done incorrectly:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Silo/LibSilo.sol#L296-L298

function transferStalk(address sender, address recipient, uint256 stalk) internal {
AppStorage storage s = LibAppStorage.diamondStorage();
uint256 roots;
roots = stalk == s.accts[sender].stalk
? s.accts[sender].roots
@> : s.sys.silo.roots.sub(1).mul(stalk).div(s.sys.silo.stalk).add(1);
​
...
}

Counter example is following: 5 * 500 / 3 is rounded down to 833. Well known formula to roundUp is to subtract 1 from numerator and add 1 to result, so in example it will be (5 * 500 - 1) / 3 + 1 = 834.

However current formula calculates (5 - 1) * 500 / 3 + 1 = 666, which is obviously wrong.

Impact

Instead of rounding up actual formula in the contrary underestimates the result.

Tools Used

Manual Review

Recommendations

roots = stalk == s.accts[sender].stalk
? s.accts[sender].roots
- : s.sys.silo.roots.sub(1).mul(stalk).div(s.sys.silo.stalk).add(1);
+ : s.sys.silo.roots.mul(stalk).sub(1).div(s.sys.silo.stalk).add(1);
Updates

Lead Judging Commences

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

Bad rounding in `LibSilo.transferStalk()`

Support

FAQs

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