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

All migrated silo users will receive less stalk accounted to their account during silo reseed

Summary

All migrated silo users will have less stalk accounted to their account during silo reseed because the s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv will not be set

Relevant GitHub Links:

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/init/reseed/L2/ReseedSilo.sol#L112

Vulnerability Details

When the Beanstalk will be migrated to an L2, the way to transfer the state from the L1 will be through the following order:

reseeds = [
reseed1, // remove all L1 diamond facets, pause beanstalk and transfer bean LPs to BCM
reseedDeployL2Beanstalk, // deploy a new beanstalk on L2
reseed3, // reseed the field component
reseed4, // reseed the barn component
reseed5, // reseed the silo component
reseed6, // reseed internal balances
reseed7, // reseed whitelisted tokens
reseed8, // reseed bean
reseed9 // add all facets to the diamond and unpause it
];

In this order, we can notice that the init from the reseed silo will be called before the reseed whitelisting of tokens.
Let's see what the reseed silo does:

function reseedSiloDeposit(SiloDeposits calldata siloDeposit) internal {
uint256 totalCalcDeposited;
uint256 totalCalcDepositedBdv;
@> uint256 stalkIssuedPerBdv = s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv;
for (uint256 i; i < siloDeposit.siloDepositsAccount.length; i++) {
AccountSiloDeposits memory deposits = siloDeposit.siloDepositsAccount[i];
uint128 totalBdvForAccount;
uint256 accountStalk;
for (uint256 j; j < deposits.dd.length; j++) {
// verify that depositId is valid.
int96 stem = deposits.dd[j].stem;
require(siloDeposit.stemTip >= stem, "ReseedSilo: INVALID_STEM");
uint256 depositId = LibBytes.packAddressAndStem(siloDeposit.token, stem);
// add deposit to account. Add to depositIdList.
s.accts[deposits.accounts].deposits[depositId].amount = deposits.dd[j].amount;
s.accts[deposits.accounts].deposits[depositId].bdv = deposits.dd[j].bdv;
s.accts[deposits.accounts].depositIdList[siloDeposit.token].push(depositId);
// increment totalBdvForAccount by bdv of deposit:
totalBdvForAccount += deposits.dd[j].bdv;
// increment by grown stalk of deposit.
accountStalk += uint96(siloDeposit.stemTip - stem) * deposits.dd[j].bdv;
// increment totalCalcDeposited and totalCalcDepositedBdv.
totalCalcDeposited += deposits.dd[j].amount;
totalCalcDepositedBdv += deposits.dd[j].bdv;
// emit events.
emit AddDeposit(
deposits.accounts,
siloDeposit.token,
stem,
deposits.dd[j].amount,
deposits.dd[j].bdv
);
emit TransferSingle(
deposits.accounts, // operator
address(0), // from
deposits.accounts, // to
depositId, // depositID
deposits.dd[j].amount // token amount
);
}
// update mowStatuses for account and token.
s.accts[deposits.accounts].mowStatuses[siloDeposit.token].bdv = totalBdvForAccount;
s.accts[deposits.accounts].mowStatuses[siloDeposit.token].lastStem = siloDeposit
.stemTip;
// increment stalkForAccount by the stalk issued per BDV.
// placed outside of loop for gas effiency.
@> accountStalk += stalkIssuedPerBdv * totalBdvForAccount;
// set stalk for account.
s.accts[deposits.accounts].stalk = accountStalk;
}
// verify that the total deposited and total deposited bdv are correct.
require(
totalCalcDeposited == siloDeposit.totalDeposited,
"ReseedSilo: INVALID_TOTAL_DEPOSITS"
);
require(
totalCalcDepositedBdv == siloDeposit.totalDepositedBdv,
"ReseedSilo: INVALID_TOTAL_DEPOSITED_BDV"
);
// set global state
s.sys.silo.balances[siloDeposit.token].deposited = siloDeposit.totalDeposited;
s.sys.silo.balances[siloDeposit.token].depositedBdv = siloDeposit.totalDepositedBdv;
}

Basically, it accounts into the Beanstalk state all the token deposits that the user has in the L1 contract. However, we can see in the third line of the function that it fetches the s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv which is a state variable of the already deployed diamond on L2, but since this silo reseed will be called before the reseed for whitelisting tokens it will be 0 because it will not be initialized yet.
This problem will not cause any revert and will harm migrated users because at the end of each user loop we can see that the accounting of the user's stalk it adds the bdv of his position multiplied by this variable that should have a value but will be 0. So the user will have less stalk accounted to his position.

Impact

High, all migrated users will get less stalk accounted to their position

Tools Used

Manual review

Recommendations

Pass the stalkIssuedPerBdv for each token in the calldata instead of fetching it from the state variable, that would make this init function to be independent of any other reseed:

struct SiloDeposits {
address token;
+ uint32 stalkIssuedPerBdv;
AccountSiloDeposits[] siloDepositsAccount;
int96 stemTip;
uint128 totalDeposited;
uint128 totalDepositedBdv;
}
function reseedSiloDeposit(SiloDeposits calldata siloDeposit) internal {
uint256 totalCalcDeposited;
uint256 totalCalcDepositedBdv;
- uint256 stalkIssuedPerBdv = s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv;
+ uint256 stalkIssuedPerBdv = siloDeposit.stalkIssuedPerBdv;
...
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Validated
Assigned finding tags:

All migrated silo users will receive less stalk during silo reseed because `s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv` is not init when `reseedSiloDeposit` is called

Appeal created

T1MOH Judge
11 months ago
draiakoo Submitter
11 months ago
inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

All migrated silo users will receive less stalk during silo reseed because `s.sys.silo.assetSettings[siloDeposit.token].stalkIssuedPerBdv` is not init when `reseedSiloDeposit` is called

Support

FAQs

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