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

`L2ContractMigrationFacet.addMigratedDepositsToAccount()` forfeits "unmowed" rewards from other Silo deposits

T1MOH

Summary

Silo deposits earn reward as time passes. Process of claiming accrued reward is called "mow": it updates current balances of Roots and Stalk associated with deposits of certain token.

L2ContractMigrationFacet is used to migrate Silo deposits owned by smart contracts. Unlike ReseedSilo, this migration is user-executed and will happen after the migration process. Problem is that such migration forfeits "unmowed" rewards of account because accumulator stem is updated to the most recent version.

Vulnerability Details

Here you can see that during migration it calculates grown Stalk associated with migrated deposits and updates lastStem in the end:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/L2ContractMigrationFacet.sol#L191

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
int96 stemTip = LibTokenSilo.stemTipForToken(depositData.token);
...
for (uint256 i; i < depositData.depositIds.length; i++) {
// verify that depositId is valid.
uint256 depositId = depositData.depositIds[i];
(address depositToken, int96 stem) = depositId.unpackAddressAndStem();
...
​
// increment by grown stalk of deposit.
@> accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
​
...
}
​
// update mowStatuses for account and token.
s.accts[account].mowStatuses[depositData.token].bdv = totalBdvForAccount;
@> s.accts[account].mowStatuses[depositData.token].lastStem = stemTip;
​
...
}

lastStem is cumulated value used to determine the value of stem at which rewards were "mowed" last time. As you can see, if user has other depositIds besides migrated ones, he looses rewards for them.

Impact

Users will lose "unmowed" rewards from other Silo deposits when they migrate via L2ContractMigrationFacet.

Tools Used

Manual Review

Recommendations

Mow in the beginning of migration:

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
+ LibSilo._mow(account, depositData.token);
int96 stemTip = LibTokenSilo.stemTipForToken(depositData.token);
uint256 stalkIssuedPerBdv = s.sys.silo.assetSettings[depositData.token].stalkIssuedPerBdv;
uint128 totalBdvForAccount;
uint128 totalDeposited;
uint128 totalDepositedBdv;
for (uint256 i; i < depositData.depositIds.length; i++) {
...
}
​
// update mowStatuses for account and token.
s.accts[account].mowStatuses[depositData.token].bdv = totalBdvForAccount;
- s.accts[account].mowStatuses[depositData.token].lastStem = stemTip;
​
...
}
Updates

Lead Judging Commences

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

`addMigratedDepositsToAccount()` forfeits "unmowed" rewards

Support

FAQs

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