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

L2ContractMigrationFacet migrates incorrect amount of Stalk

T1MOH

Summary

L2ContractMigrationFacet is used to migrate Silo deposits owned by smart contracts. During migration it calculates grown Stalk associated with deposits. Problem is that wrong precision is used in that calculation.

Vulnerability Details

Here you can see that it just multiplies stem delta and BDV:

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
...
for (uint256 i; i < depositData.depositIds.length; i++) {
...
​
// increment by grown stalk of deposit.
@> accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
​
...
}
​
...
}

However correct way to calculate grown Stalk is defined in LibSilo.stalkReward(), i.e. it must divide result by 1e6:
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/df2dd129a878d16d4adc75049179ac0029d9a96b/protocol/contracts/libraries/Silo/LibSilo.sol#L650-L669

/**
* @notice Calculates the Stalk reward based on the start and end
* stems, and the amount of BDV deposited. Stems represent the
* amount of grown stalk per BDV, so the difference between the
* start index and end index (stem) multiplied by the amount of
* bdv deposited will give the amount of stalk earned.
* formula: stalk = bdv * (ΔstalkPerBdv)
*
* @dev endStem must be larger than startStem.
*
*/
function stalkReward(
int96 startStem,
int96 endStem,
uint128 bdv
) internal pure returns (uint256) {
uint128 reward = uint128(uint96(endStem.sub(startStem))).mul(bdv).div(PRECISION);
​
return reward;
}

Impact

Grown Stalk associated with migrated deposits owned by smart contracts will be calculated incorrectly, i.e. it overestimates actual Stalk by 1e6 times. Stalk is governance token, hence governance will be compromised.

Additionally migrated deposits will receive much higher amount of Roots, unintentionally stealing it from new depositors. New depositors will receive dust amount of Roots.

Tools Used

Manual Review

Recommendations

function addMigratedDepositsToAccount(
address account,
AccountDepositData calldata depositData
) internal returns (uint256 accountStalk) {
...
for (uint256 i; i < depositData.depositIds.length; i++) {
...
​
// increment by grown stalk of deposit.
- accountStalk += uint96(stemTip - stem) * depositData.bdvs[i];
+ accountStalk += LibSilo.stalkReward(stem, siloDeposit.stemTip, deposits.dd[j].bdv);
​
...
}
​
...
}
Updates

Lead Judging Commences

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

L2ContractMigrationFacet migrates incorrect amount of Stalk should be divided by 1e6

Appeal created

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

L2ContractMigrationFacet migrates incorrect amount of Stalk should be divided by 1e6

Support

FAQs

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