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

Incorrect State Update in `addMigratedDepositsToAccount` Function

0xbeastboy

Summary

The redeemDepositsAndInternalBalances function in the L2ContractMigrationFacet contract is designed to facilitate the migration of deposits and internal balances from an L1 address to an L2 address. However, the function has an issue with incorrect state updates in the addMigratedDepositsToAccount function, leading to potential inaccuracies in the system's accounting of deposited amounts.

Vulnerability Details

The addMigratedDepositsToAccount function is responsible for adding migrated deposits to a user's account and updating the global state. The function updates the global state variables s.sys.silo.balances[depositData.token].deposited and s.sys.silo.balances[depositData.token].depositedBdv outside the loop that processes each deposit. However, the values of totalDeposited and totalDepositedBdv are not correctly aggregated within the loop (just declared before the loop), resulting in incorrect state updates.

See the following code:

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]);
}
​
// 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;
​
// increment stalkForAccount by the stalk issued per BDV.
// placed outside of loop for gas effiency.
accountStalk += stalkIssuedPerBdv * totalBdvForAccount;
}

Impact

Incorrect aggregation of totalDeposited and totalDepositedBdv` within the loop can lead to inaccurate accounting of the total deposited amounts. This discrepancy can affect the integrity of the overall system and lead to potential financial inaccuracies, undermining the reliability and trustworthiness of the migration process.

Tools Used

Manual Review

Recommendations

To address the issue of incorrect state updates, the function should correctly aggregate the values of totalDeposited and totalDepositedBdv within the loop that processes each deposit. This ensures accurate accounting of the deposited amounts.

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.