Beanstalk Notion
Beanstalk Notion
/
🪲
Bug Reports
/
BIC Notes
/
📄
Report #34357
📄

Report #34357

Report Date
August 9, 2024
Status
Confirmed
Payout
3,000

`_setReserves()` fails when supporting Well implementation with an odd number of tokens

‣
Report Info

Report ID

#34357

Report type

Smart Contract

Has PoC?

Yes

Target

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

Impacts

  • Permanent freezing of funds
  • Contract fails to deliver promised returns, but doesn't lose value

Description

In a Well.sol implementation with an odd number of tokens, _setReserves() will always revert after the first successful call, due to faulty implementation of the LibBytes.storeUint128() function.

Root Cause

The _setReserves() function is used to both verify and store the token balances of the Well.sol deployment for all external functions (i.e. swapTo(), swapFrom(), ...). It does so by first looping through all of the tokens, and comparing the stored token balance (in reserves) with the actual token balance (obtained by balanceOf()). It does not cause any issues when there are two tokens supported by the Well.sol implementation, which is how it is currently deployed.

However, in a Well.sol deployment that supports an odd number of tokens (I.e. 3) there is a permanent skewing of the internal contract balance stored in reserves and the actual token balance of the contract obtained by balanceOf(). It is caused by a faulty implementation of the _setReserves() functionality; more specifically, the LibBytes.storeUint128() function. The memory storage implemented by LibBytes.storeUint128() does not properly function when the Well.sol implementation contains and odd number of tokens, and the wrong token value is copied and saved for the final token.

Impact

  • Most external functions will always revert when called

The main impact of the vulnerability is that in a Well.sol deployment with an odd number of tokens, no function will be able to execute successfully, and will always revert. This is because most external functions available for users to call all contain the _setReserves() function to verify the token balance. Because of the incorrect storage, _setReserves() will always revert when verifying the stored token balance against the actual balance.

POC

The POC was created in the [Github directory](https://github.com/BeanstalkFarms/Basin/tree/master), as currently, a Well.sol deployment with an odd number of tokens does not exist. Specifically, it is designed as a forge test in the /test directory.

Here are the steps performed in the POC:

  • First, a Well.sol deployment with 3 tokens is initialized.
  • Then, a simple swap occurs between the first two tokens, with the amount out being 123*10^18.
  • Then, a second swap is attempted. However, it will revert for the reasons mentioned above.

For the code, see the POC section. To run the POC, create a test in the test directory (i.e. {test_name}.t.sol) and run the command forge test -vv --match-path test/{test_name}.t.sol from the main directory.

To see the logs mentioned in the Expected Results section, uncomment the piece of code that says: "Uncomment to see logs".

Expected Results

The results of the POC are shown below. There are two different results: one with the logs enabled, and one without.

No Logs:

[â ’] Compiling...
[â °] Compiling 1 files with Solc 0.8.20
[â ”] Solc 0.8.20 finished in 7.28s
Compiler run successful!

Ran 1 test for test/Well.POC.t.sol:WellPOC
[PASS] test_POC(uint256) (runs: 257, μ: 184963, ~: 184963)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 171.48ms (158.01ms CPU time)

Ran 1 test suite in 181.07ms (171.48ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

With Logs Enabled:

Explanation

The second swap will fail due to an InvalidReserves() revert, as demonstrated by the expectRevert() check in the POC. This is because the internal balances of the contract (in reserves) does not match the actual balances in the contract (obtained by balanceOf()). In particular, the former is logged as the Well Token 1 Balance, Well Token 2 Balance, and Well Token 3 Balance, while the latter is logged in the lines beginning with [i] Before Swap 2: Well Reserve. We can see that the balance of Token 2 does not match after the swap, causing the revert.

Recommended Mitigation

Modify the padding applied to the token reserve when there is an odd number of tokens. More specifically, the code on line 54 of LibBytes.sol:

... add(mload(add(reserves, add(iByte, 32))), shr(128, shl(128, sload(add(slot, maxI)))))

Using a simple padding with the first reserve like so:

... add(mload(add(reserves, add(iByte, 32))), shr(128, shl(128, mload(add(reserves, 0)))))

resolves the vulnerability.

References

https://github.com/BeanstalkFarms/Basin/blob/58ca5971061d7a02fa0fab3ee16297391bcb97e8/src/Well.sol#L718

Proof of concept

This POC was created for the Basin Github Directory. To run the POC, create a test in the test directory (i.e. {test_name}.t.sol) and run the command forge test -vv --match-path test/{test_name}.t.sol from the main directory.

BIR-19: Odd Token Wells

BIC Response

After some investigation we have confirmed this is a valid bug report. The issue has been resolved in our codebase here https://github.com/BeanstalkFarms/Basin/pull/136/files

As outlined in the program, for Medium severity reports, the BIC determines a reward between 1k and 10k Beans based on:

The exploitability of the bug; The impact it causes; and The likelihood of the vulnerability presenting itself.

Given that this issue is not exploitable, and would likely be caught in preliminary testing before any 3+ token Well were deployed on-chain, the BIC has determined this report to be awarded 3,000 Beans.

Logs:
  Before Swap 1 Well Token 1 Balance: 1000000000000000000000
  Before Swap 1 Well Token 2 Balance: 1000000000000000000000
  Before Swap 1 Well Token 3 Balance: 1000000000000000000000

  [i] Number of tokens in Well: 3
    Token 3: {1000000000000000000000}

  [*] Executing Swap 1
  Well Token 1 Balance: 1140250855188141391107

  Well Token 2 Balance: 877000000000000000000

  Well Token 3 Balance: 1000000000000000000000


User Token 1 Balance: 859749144811858608893

  User Token 2 Balance: 1123000000000000000000

  User Token 3 Balance: 1000000000000000000000

  [i] Before Swap 2: Well Reserve
    Token 0: {1140250855188141391107}
    Token 1: {877000000000000000000}
    Token 2: {2000000000000000000000}

  [!] Notice how the reserve of Token 3 does not match the Well Token 3 balance. This will cause _setReserves() to fail and revert every time:
  [*] Attempting Swap 2

  [X] Expect Reverts Due to InvalidReserves()

  Error: a == b not satisfied [uint]
        Left: 123000000000000000000
       Right: 0

Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 47.52ms (19.16ms CPU time)

Ran 1 test suite in 166.00ms (47.52ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)

Failing tests:
Encountered 1 failing test in test/Well.POC.t.sol:WellPOC
[FAIL. Reason: assertion failed; counterexample: calldata=0x20e9e87f000000000000000000000000000000000000000000000000000000003dd114f2 args=[1037112562 [1.037e9]]] test_POC(uint256) (runs: 0, μ: 0, ~: 0)
Encountered a total of 1 failing tests, 0 tests succeeded
// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {IERC20, Balances, Call, MockToken, Well} from "test/TestHelper.sol";
import {SwapHelper, SwapAction} from "test/SwapHelper.sol";
import {MockFunctionBad} from "mocks/functions/MockFunctionBad.sol";
import {IWellFunction} from "src/interfaces/IWellFunction.sol";
import {IWell} from "src/interfaces/IWell.sol";
import {IWellErrors} from "src/interfaces/IWellErrors.sol";
import "forge-std/console.sol";


contract WellPOC is SwapHelper {
    function setUp() public {
        //@audit Setup a Well deployment with 3 tokens
        setupWell(3);
    }



    /// @dev tests assume 2 tokens in future we can extend for multiple tokens
    function test_POC(uint256 amountOut) public prank(user) {
        // User has 1000 of each token
        // Given current liquidity, swapping 1000 of one token gives 500 of the other
        uint256 maxAmountIn = 1000 * 1e18;
        uint256[] memory well_reserves;
        amountOut = 123 * 1e18;


        //@audit State before any swaps have occured
        Balances memory wellBalancesBefore = getBalances(address(well), well);

        console.log("Before Swap 1 Well Token 1 Balance: %d", wellBalancesBefore.tokens[0]);
        console.log("Before Swap 1 Well Token 2 Balance: %d", wellBalancesBefore.tokens[1]);
        console.log("Before Swap 1 Well Token 3 Balance: %d\n", wellBalancesBefore.tokens[2]);

        console.log("[i] Number of tokens in Well: %d\n", well.numberOfTokens());

        well_reserves = well.getReserves();
        console.log("[i] Before Swap 1: Well Reserve\n    Token 1: {%d}\n    Token 2: {%d}\n    Token 3: {%d}\n", well_reserves[0], well_reserves[1], well_reserves[2]);
        console.log("[*] Executing Swap 1");

        //@audit Execute Swap 1
        well.swapTo(tokens[0], tokens[1], maxAmountIn, amountOut, user, type(uint256).max);

        Balances memory userBalancesAfter = getBalances(user, well);
        Balances memory wellBalancesAfter = getBalances(address(well), well);
        console.log("Well Token 1 Balance: %d\n", wellBalancesAfter.tokens[0]);
        console.log("Well Token 2 Balance: %d\n", wellBalancesAfter.tokens[1]);
        console.log("Well Token 3 Balance: %d\n", wellBalancesAfter.tokens[2]);

        console.log("\nUser Token 1 Balance: %d\n", userBalancesAfter.tokens[0]);
        console.log("User Token 2 Balance: %d\n", userBalancesAfter.tokens[1]);
        console.log("User Token 3 Balance: %d\n", userBalancesAfter.tokens[2]);

        well_reserves = well.getReserves();
        console.log("[i] Before Swap 2: Well Reserve\n    Token 0: {%d}\n    Token 1: {%d}\n    Token 2: {%d}\n", well_reserves[0], well_reserves[1], well_reserves[2]);
        console.log("[!] Notice how the reserve of Token 3 does not match the Well Token 3 balance. This will cause _setReserves() to fail and revert every time: ");

        console.log("[*] Attempting Swap 2\n");

        //@audit The next swap will fail due to the storage and actual discrepancy
        vm.expectRevert(IWellErrors.InvalidReserves.selector);
        well.swapTo(tokens[0], tokens[1], maxAmountIn, amountOut, user, type(uint256).max);
        console.log("[X] Expect Reverts Due to InvalidReserves()\n");

        //@audit Uncomment to see logs:
        //assertEq(amountOut, 0);
        //@audit End
    }

}