DeFiHardhatFoundry
250,000 USDC
View results
Submission Details
Severity: high
Valid

`totalDeposited` isn't accounted properly.

## Line of code
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/L2ContractMigrationFacet.sol#L152
## Vulnerability detail
The function `L2ContractMigrationFacet::addMigratedDepositsToAccount()` is used to migrate the deposits to the accounts. The function takes two parameters: account and depositData. The problem in the function is that the `totalDeposited` count isn't updated in the for loop when going through each one of the deposits. After completing the for loop, we keep track of the `totalDeposited` as global state in the line `s.sys.silo.balances[depositData.token].deposited = totalDeposited;` but as it is not updated in the loop, the `totalDeposited` will be always zero irrespective of adding deposits.
```solidity
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]);
// @audit issue total deposit isn't updated in the loop. [H]
}
// 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
The global state for `totalDeposited` amount will be incorrect and zero.
## Tools Used
Manual Review
## Recommendation
Update the `totalDeposited` in the loop.
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.