Omniscia Beanstalk Audit
UpdateSilo Manual Review Findings
UpdateSilo Manual Review Findings
USO-01M: Potential Truncation of Unclaimed Root Accounting
Type | Severity | Location |
---|---|---|
Mathematical Operations | UpdateSilo.sol:L79, L83 |
Description:
The legacy farming of beans for the V2 implementation uses the concept of unclaimed roots, however, all the calculations utilise fractional operations that are prone to truncation and as such the unclaimed roots of a particular account can exceed the total unclaimed roots for the last person to claim them.
Example:
protocol/contracts/farm/facets/SiloFacet/UpdateSilo.sol
72function farmLegacyBeans(address account, uint32 update) private {73 uint256 beans;74 if (update < s.hotFix3Start) {75 beans = balanceOfFarmableBeansV1(account);76 if (beans > 0) s.v1SI.beans = s.v1SI.beans.sub(beans);77 }78
79 uint256 unclaimedRoots = balanceOfUnclaimedRoots(account);80 uint256 beansV2 = balanceOfFarmableBeansV2(unclaimedRoots);81 beans = beans.add(beansV2);82 if (beansV2 > 0) s.v2SIBeans = s.v2SIBeans.sub(beansV2);83 s.unclaimedRoots = s.unclaimedRoots.sub(unclaimedRoots);84 s.a[account].lastSIs = s.season.sis;85
86 uint256 seeds = beans.mul(C.getSeedsPerBean());87 s.a[account].s.seeds = s.a[account].s.seeds.add(seeds);88 s.a[account].s.stalk = s.a[account].s.stalk.add(beans.mul(C.getStalkPerBean()));89 addBeanDeposit(account, season(), beans);90}
Recommendation:
We advise a similar paradigm to balanceOfFarmableBeansV2
to be imposed whereby the total unclaimed roots are simply set to zero if the account's unclaimed roots exceed them.
Alleviation:
The same paradigm as balanceOfFarmableBeansV2
has been applied to the relevant statements as advised via a ternary operator.