Report #26070

Report Date

November 23, 2023

Status

Closed

Payout

Attacker can front run user transactions and steal user funds

‣

Report Info

Report ID

#26070

Report type

Smart Contract

Has PoC?

Yes

Target

https://etherscan.io/address/0xBA510e11eEb387fad877812108a3406CA3f43a4B

Impacts

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

Bug Description

Although the shift () is used when doing a multi-hop swap in 1 single transaction to reduce costs by shifting tokens from one Well to another rather than returning them to the multicall router.

The same shift() function can be exploited by malicious actors to steal user funds because of its weak access controls.

 function shift(
        IERC20 tokenOut,
        uint256 minAmountOut,
        address recipient
    ) external nonReentrant returns (uint256 amountOut) {
        IERC20[] memory _tokens = tokens();
        uint256 tokensLength = _tokens.length;
        _updatePumps(tokensLength);

The malicious attacker can front run transactions being carried out by users who are using the router + shift() route and steal user tokens.

The same function can be exploited by attacker because the inputs are not validated and they are processed without confirming the source or validity of the inputed parameters

Impact

Critical as user funds are lost

Risk Breakdown

Difficulty to Exploit: Easy

Weakness: High

CVSS2 Score: 10

Recommendation

Access modifier or require statement whereby the shift() function can only be called by the router

function shift(
        IERC20 tokenOut,
        uint256 minAmountOut,
        address recipient
    ) external onlyRole(Router) nonReentrant returns (uint256 amountOut) {

require (isAdmin(Router), "must have admin privileges" )
}

References

https://github.com/BeanstalkFarms/Basin/blob/master/src/Well.sol#L381-L489

Proof of concept

Step a- A User named bobs utilizes the multi-hop swap of the pipeline multi-call contract.

Step b- Bob engages the router + Shift route to save gas

This router + shift route engages the balances of the pool whereby if there is a change in token balances relative to the currently stored reserves, the extra tokens are shifted into tokenOut.

Step C - unknown to bob an attacker who is monitoring the well transactions gains advantage of the changes in token balances and invokes the shift() function also passing an IERC20 token similar to bobs own with their own address as recipient to steal user funds.

** Attacker can exploit this vulnerability perpetually under the nose of the protocol and continues to siphon user funds.**

This POC test that shows an attacker can advantage of the changes in token balances to steal user funds

function test_shift_tokenOut() public prank(user) {
        amount = bound(amount, 1, 1000e18);

        // Transfer `amount` of token0 to the Well
        tokens[0].transfer(address(well), amount);
        Balances memory wellBalanceBeforeShift = getBalances(address(well), well);
        assertEq(wellBalanceBeforeShift.tokens[0], 1000e18 + amount, "Well should have received tokens");

        // Get a user with a fresh address (no ERC20 tokens)
        address _user = users.getNextUserAddress();
        Balances memory userBalanceBeforeShift = getBalances(_user, well);

        // Verify that the user has no tokens
        assertEq(userBalanceBeforeShift.tokens[0], 0, "User should start with 0 of token0");
        assertEq(userBalanceBeforeShift.tokens[1], 0, "User should start with 0 of token1");

        uint256 minAmountOut = well.getShiftOut(tokens[0]);
        uint256[] memory calcReservesAfter = new uint256[](2);
        calcReservesAfter[0] = tokens[0].balanceOf(address(well)) - minAmountOut;
        calcReservesAfter[1] = tokens[1].balanceOf(address(well));

        vm.expectEmit(true, true, true, true);
        emit Shift(calcReservesAfter, tokens[0], minAmountOut, _user);
        // Shift the imbalanced token as the token out
        well.shift(tokens[0], 0, _user);

        uint256[] memory reserves = well.getReserves();
        Balances memory userBalanceAfterShift = getBalances(_user, well);
        Balances memory wellBalanceAfterShift = getBalances(address(well), well);

        // User should have gained token0
        assertEq(userBalanceAfterShift.tokens[0], amount, "User should have gained token0");
        assertEq(
            userBalanceAfterShift.tokens[1], userBalanceBeforeShift.tokens[1], "User should NOT have gained token1"
        );

BIC Response

This is not a valid bug report because shift should be used with a multicall contract, rather than two individual transactions. The report and PoC describe user error / intended behavior.

Due to these reasons, we are closing the submission and no reward will be issued.