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

`L2ContractMigrationFacet.addMigratedDepositsToAccount()` forgets to calculate `totalDeposited` and `totalDepositedBdv`

T1MOH

Summary

In the end of Silo deposits migration it sets s.sys.silo.balances, however those totalDeposited and totalDepositedBdv were never updated in the loop and therefore are 0:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/beanstalk/silo/L2ContractMigrationFacet.sol#L152-L200

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
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++) {
// verify that depositId is valid.
uint256 depositId = depositData.depositIds[i];
(address depositToken, int96 stem) = depositId.unpackAddressAndStem();
require(depositToken == depositData.token, "Migration: INVALID_DEPOSIT_ID");
require(stemTip >= stem, "Migration: INVALID_STEM");
​
// add deposit to account.
s.accts[account].deposits[depositId].amount = depositData.amounts[i];
s.accts[account].deposits[depositId].bdv = depositData.bdvs[i];
​
// increment totalBdvForAccount by bdv of deposit:
totalBdvForAccount += depositData.bdvs[i];
​
// increment by grown stalk of deposit.
accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
​
// emit events.
emit AddDeposit(
account,
depositData.token,
stem,
depositData.amounts[i],
depositData.bdvs[i]
);
emit TransferSingle(msg.sender, address(0), account, depositId, depositData.amounts[i]);
}
​
...
​
// set global state
@> s.sys.silo.balances[depositData.token].deposited = totalDeposited;
@> s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
​
...
}

Impact

Global values of token balances in Silo will be incorrect. It will result in:

  1. Incorrect calculation of Gauge points, because s.sys.silo.balances used in this module.

  2. Incorrect check of invariants in Invariable because written balances are lower than actual amount. So when there will be shortage of tokens, Invariable.sol will not halt protocol.

Tools Used

Manual Review

Recommendations

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
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++) {
...
​
// add deposit to account.
s.accts[account].deposits[depositId].amount = depositData.amounts[i];
s.accts[account].deposits[depositId].bdv = depositData.bdvs[i];
​
// increment totalBdvForAccount by bdv of deposit:
totalBdvForAccount += depositData.bdvs[i];
+ totalDeposited += depositData.amounts[i];
+ totalDepositedBdv += depositData.bdvs[i];
​
...
}
​
...
​
// set global state
s.sys.silo.balances[depositData.token].deposited = totalDeposited;
s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
​
...
}
Updates

Lead Judging Commences

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

`addMigratedDepositsToAccount` Function doesn't properly aggregate the totalDeposited and totalDepositBdved

Support

FAQs

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