📄

Report #13489

Report Date
November 13, 2022

Multiple Withdraw with (External/External-Internal) mode

Report Info

Report ID

#13489

Target

SiloFacet.sol (Out of scope)

Report type

Smart Contract

Impacts

Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

Has PoC?

Yes

Bug Description

Hopefully this is not a bug and just a Foundry test issue or the way Diamond contracts are set up but I tried with vm.roll(-different block-) and same results. Beanstalk has world class developers and a mistake like this is highly unlikely but things happen. Here is the full analysis:

SiloFacet.sol allows for multiple withdrawals of a deposit made with External or External-Internal mode with function, if withdrawal is made within the same season: function deposit( address token, uint256 amount, LibTransfer.From mode ) external;

Withdrawals are made with the function that follows after that: function withdrawDeposit( address token, uint32 season, uint256 amount ) external;

Not sure why this is allowed or test are passing (still investigating).

Impact

If users are able to withdraw more than they deposit the harm is obvious hence I called it a Locust Plaque. However, if both Silo and user balances are unaffected than multiple withdrawals is just a result of bad design and should be address.

Risk Breakdown

Difficulty to Exploit: Easy Weakness: CVSS2 Score:

Recommendation

If this is not a bug and a design issue, a deposit should only be withdrawn once, there should be a check that informs users that withdrawals have already been made.

Proof of concept

<code> // SPDX-License-Identifier: MIT pragma solidity 0.8.10;

import "forge-std/Test.sol"; import "forge-std/console.sol";

/*Key Information Attack Name : Locust Plague Summary : Seems there is multiple withdrawals possible with direct deposit of tokens using modes External and External-Internal.

Risk Assessment : CriticalVulnerable Contract : SiloFacet.sol Vulnerable Function(s) : Function withdrawDeposit Vulnerable Type : Multiple Call Tools Used : Sol2UML, VS-Surya, Foundry and Slither Research links of Vulnerability : N/A

POC-Written by : Sentient-X twitter : @sentient_x

Disclaimer - This POC is the product of the authors best understanding of the project and associated vulnerability all claims to be proven. */

address constant BEAN_POOL = 0xc9C32cd16Bf7eFB85Ff14e0c8603cc90F6F2eE49; //CURVE_BEAN_METAPOOL address constant DIAMOND = 0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5;

address constant BEAN3CRV = 0xc9C32cd16Bf7eFB85Ff14e0c8603cc90F6F2eE49; //CURVE_BEAN_METAPOOL

//Bean tokens address constant BEAN_LP = 0x1BEA3CcD22F4EBd3d37d731BA31Eeca95713716D; //urBEAN3CRV address constant BEAN = 0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab; //BEAN Token address constant urBEAN = 0x1BEA0050E63e05FBb5D8BA2f10cf5800B6224449; //urBEAN Token

address constant Crv = 0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490; //Curve.fi DAI/USDC/USDT (3Crv) address constant USDC = 0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48;

contract ContractTest is Test { ICurve private constant pool = ICurve(BEAN_POOL); IDIAMOND private constant diamond = IDIAMOND(DIAMOND);

IERC20 public constant lpToken = IERC20(BEAN_LP); IERC20 public constant bean = IERC20(BEAN); IERC20 public constant urbean = IERC20(urBEAN); IERC20 public constant crv = IERC20(Crv); IERC20 public constant bean2 = IERC20(BEAN3CRV); IERC20 public constant usdc = IERC20(USDC);

address alice = vm.addr(1);

function setUp() public { vm.createSelectFork("mainnet",15948123); //fork mainnet at block 15533430 }

function testselfdestruct() external payable {

//deal whitelisted token here (bean) deal(address(bean), address(alice), 10000e18);

vm.startPrank(alice); bean.approve(address(diamond), type(uint).max); diamond.deposit(address(bean), 10000e18, 0); //Direct deposit of whitelisted token (bean)

//initiate multiple withdrawals here diamond.withdrawDeposit(address(bean), 8402, 1);

vm.roll(15948923);

diamond.withdrawDeposit(address(bean), 8402, 1); diamond.withdrawDeposit(address(bean), 8402, 1); diamond.withdrawDeposit(address(bean), 8402, 1); diamond.withdrawDeposit(address(bean), 8402, 1);

vm.stopPrank();

}

}

interface ICurve { function get_virtual_price() external view returns (uint);

function remove_liquidity(uint lp, uint[2] calldata min_amounts) external returns (uint[2] memory);

function add_liquidity(uint[2] calldata _amounts, uint _min_mint_amount) external payable returns (uint);

function remove_liquidity_one_coin( uint lp, int128 i, uint min_amount ) external returns (uint);

function exchange_underlying(int128 i, int128 j, uint256 dx, uint256 min_dy) external; }

interface IERC20 { function totalSupply() external view returns (uint);

function balanceOf(address account) external view returns (uint);

function transfer(address recipient, uint amount) external returns (bool);

function allowance(address owner, address spender) external view returns (uint);

function approve(address spender, uint amount) external returns (bool);

function transferFrom( address sender, address recipient, uint amount ) external returns (bool);

event Transfer(address indexed from, address indexed to, uint value); event Approval(address indexed owner, address indexed spender, uint value); }

interface IDIAMOND { function approveDeposit(address token, address account, uint256 amount) external; function deposit(address token ,uint256 amount , uint8 mode) external; function withdrawDeposit(address token, uint32 season, uint256 amount) external; function updateSilo(address account) external; function convert(bytes calldata convertData, uint32[] memory crates, uint256[] memory amounts) external; function withdrawDeposits(address token, uint32[] calldata seasons, uint256[] calldata amounts) external; function buyAndDepositBeans(uint256 amount, uint256 buyAmount) external; function plant() external; function enrootDeposit(address token, uint32 _season, uint256 amount) external; function harvest(uint256[] calldata plots) external; function mintFertilizer(uint128 amount, uint256 minLP, uint8 mode) external; function chop(address token, uint256 amount, uint8, uint8) external;

} </code>

Reporter Response

Apologies made a mistake, the withdrawals are for 1 not the total amounts please cancel this report. I mistaken the 1 for transfer mode but its actually the amount and thats why the test was passing. It has been a long night, sorry

Immunefi Reponse

Hi, Immunefi has reviewed this vulnerability report and decided to close since being out of scope for Beanstalk bug bounty program.
  • claimed impact by the whitehat is in scope for the bug bounty program
  • claimed asset by the whitehat is not in scope for the bug bounty program
  • PoC has been submitted to the project
  • claimed severity is in scope for the bug bounty program

Since this bug bounty program does not require Immunefi's triaging, note that Immunefi does not:

  • check if whitehat's claims are factually correct
  • check PoC to understand the validity
  • assess the submission's severity

These activities are the project's responsibility.

The project will now be automatically subscribed and receive a report of the closed submission and can evaluate if they are interested in re-opening it. However, note that they are not under any obligation to do so.