Github link
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/9c7b9fd521ad7cbe65cc788df181887c0eb39c6d/protocol/contracts/beanstalk/silo/L2ContractMigrationFacet.sol#L194
Summary
While migrating deposits to L2, addMigratedDepositsToAccount() doesn't increase the global deposited and depositedBdv balances.
Vulnerability Details
Users can redeem their deposits and internal balances on L2 using redeemDepositsAndInternalBalances().
And redeemDepositsAndInternalBalances() calls addMigratedDepositsToAccount() to manage each deposit.
But in addMigratedDepositsToAccount(), it doesn't track totalDeposited and totalDepositedBdv at all.
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++) {
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");
s.accts[account].deposits[depositId].amount = depositData.amounts[i];
s.accts[account].deposits[depositId].bdv = depositData.bdvs[i];
totalBdvForAccount += depositData.bdvs[i];
accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
emit AddDeposit(
account,
depositData.token,
stem,
depositData.amounts[i],
depositData.bdvs[i]
);
emit TransferSingle(msg.sender, address(0), account, depositId, depositData.amounts[i]);
}
s.accts[account].mowStatuses[depositData.token].bdv = totalBdvForAccount;
s.accts[account].mowStatuses[depositData.token].lastStem = stemTip;
s.sys.silo.balances[depositData.token].deposited = totalDeposited;
s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
accountStalk += stalkIssuedPerBdv * totalBdvForAccount;
}
So the global deposited and depositedBdv won't be increased after the migration.
These global balances are used in several logics like the token withdrawal validation and gauge points calculations.
As an example, these balances are decreased during a withdrawal and some withdrawals would revert due to underflow as these global balances are less than the total balances of all users.
Impact
Several logics with the withdrawal wouldn't work properly with the wrong global balances.
Tools Used
Manual Review
Recommendations
We should increase the global balances in addMigratedDepositsToAccount().
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];
+ totalDeposited += depositData.amounts[i];
+ totalDepositedBdv += 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]);
}
// update mowStatuses for account and token.
s.accts[account].mowStatuses[depositData.token].bdv = totalBdvForAccount;
s.accts[account].mowStatuses[depositData.token].lastStem = stemTip;
// set global state
- s.sys.silo.balances[depositData.token].deposited = totalDeposited;
- s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
+ s.sys.silo.balances[depositData.token].deposited = totalDeposited;
+ s.sys.silo.balances[depositData.token].depositedBdv = totalDepositedBdv;
// increment stalkForAccount by the stalk issued per BDV.
// placed outside of loop for gas effiency.
accountStalk += stalkIssuedPerBdv * totalBdvForAccount;
}
