Tapioca DAO - kutugu's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L192

Vulnerability details

Impact

BigBang only considers totalBorrow.elastic == 0 and totalBorrow.elastic > _maxDebtPoint case, but does not take into account 0 <totalBorrow.elastic < debtStartPoint. When this happens, the subtraction will overflow, causing getDebtRate and _accrue revert, and since it is an interest calculation, it will DOS the lending, withdrawing, and clearing operations of the relevant markets.

Proof of Concept

    function getDebtRate() public view returns (uint256) {
        if (_isEthMarket) return penrose.bigBangEthDebtRate(); // default 0.5%
        if (totalBorrow.elastic == 0) return minDebtRate;

        uint256 _ethMarketTotalDebt = BigBang(penrose.bigBangEthMarket())
            .getTotalDebt();
        uint256 _currentDebt = totalBorrow.elastic;
        uint256 _maxDebtPoint = (_ethMarketTotalDebt *
            debtRateAgainstEthMarket) / 1e18;

        if (_currentDebt >= _maxDebtPoint) return maxDebtRate;

        // @audit may revert here
        uint256 debtPercentage = ((_currentDebt - debtStartPoint) *
            DEBT_PRECISION) / (_maxDebtPoint - debtStartPoint);
        uint256 debt = ((maxDebtRate - minDebtRate) * debtPercentage) /
            DEBT_PRECISION +
            minDebtRate;

        if (debt > maxDebtRate) return maxDebtRate;

        return debt;
    }

Tools Used

Manual review

Recommended Mitigation Steps

if (totalBorrow.elastic < debtStartPoint) return minDebtRate;

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/compound/CompoundStrategy.sol#L115 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/compound/CompoundStrategy.sol#L159

Vulnerability details

Impact

Note that this also applies to other strategy contracts: When currentBalance is calculated, the view method is used to obtain exchangeRate, which is stale value.
As a result, the user will burn more share in the withdraw, and the amount of extra redeem will not be transferred to the user, but only the amount of input will be transferred, resulting in the extra funds being stuck in the contract forever.

Proof of Concept

    function _currentBalance() internal view override returns (uint256 amount) {
        uint256 shares = cToken.balanceOf(address(this));
        uint256 pricePerShare = cToken.exchangeRateStored();
        uint256 invested = (shares * pricePerShare) / (10 ** 18);
        uint256 queued = wrappedNative.balanceOf(address(this));
        return queued + invested;
    }

The stale exchangeRate value will cause two problems:

1. The currentBalance calculated is inaccurate, may DOS withdraw:

• Actual exchangeRate is lower than view exchangeRate: currentBalance is undervalued, lower than amount and can't be withdrew
• Actual exchangeRate is greater than view exchangeRate: currentBalance is overvalued, greater than amount, but actual balance is lower than amount and transfer fail.

	function _withdraw(
        address to,
        uint256 amount
    ) internal override nonReentrant {
        uint256 available = _currentBalance();
		// @audit undervalue currentBalance may revert
        require(available >= amount, "CompoundStrategy: amount not valid");

        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            uint256 pricePerShare = cToken.exchangeRateStored();
            uint256 toWithdraw = (((amount - queued) * (10 ** 18)) /
                pricePerShare);

            cToken.redeem(toWithdraw);
            INative(address(wrappedNative)).deposit{
                value: address(this).balance
            }();
        }

        require(
            wrappedNative.balanceOf(address(this)) >= amount,
            "CompoundStrategy: not enough"
        );
		// @audit overvalue currentBalance may revert
        wrappedNative.safeTransfer(to, amount);

        emit AmountWithdrawn(to, amount);
    }

2. The toWithdraw calculated is inaccurate, may result in loss of funds:

• Actual exchangeRate is lower than view exchangeRate: toWithdraw is undervalued, actual balance is lower than amount and transfer fail.
• Actual exchangeRate is greater than view exchangeRate: toWithdraw is overvalued, user's excess balance is stuck in the contract and cannot be withdraw.

	function _withdraw(
        address to,
        uint256 amount
    ) internal override nonReentrant {
        uint256 available = _currentBalance();
        require(available >= amount, "CompoundStrategy: amount not valid");

        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            uint256 pricePerShare = cToken.exchangeRateStored();
            uint256 toWithdraw = (((amount - queued) * (10 ** 18)) /
                pricePerShare);

            cToken.redeem(toWithdraw);
            INative(address(wrappedNative)).deposit{
                value: address(this).balance
            }();
        }

        require(
            wrappedNative.balanceOf(address(this)) >= amount,
            "CompoundStrategy: not enough"
        );
		// @audit undervalue toWithdraw may revert
		// @audit overvalue toWithdraw may loss funds, excess balance is stuck in the contract
        wrappedNative.safeTransfer(to, amount);

        emit AmountWithdrawn(to, amount);
    }

Since there are many scenarios involved, I will only describe the most impactful scene of users' funds loss:

1. Users borrow money in CEther market and generate interest. As time goes by, the interest increases gradually and the underlying value of share increases, that is, exchangeRate increases gradually
2. At this point, a user redeem tokens through CompoundStrategy, It should first update the interest by exchangeRateCurrent/accrueInterest and use the latest exchangeRate for subsequent operations.
3. But contracts read exchangeRateStored directly, which is lower than it actually is, because of not account for interest. And toWithdraw is higher than the actual value, so the user needs to burn more share and redeem more WEther.
4. In the end, the amount of WEther transdered to the user is not the actual redeem amount, and the excess balance is stuck in the contract. After users withdraw, the remaining funds are stuck in the contract forever.

Tools Used

Manual review

Recommended Mitigation Steps

Use exchangeRateCurrent instead of exchangeRateStored

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tap-token-audit/blob/59749be5bc2286f0bdbf59d7ddc258ddafd49a9f/contracts/tokens/TapOFT.sol#L225

Vulnerability details

Impact

According to the code comments, emissionForWeek is the amount of emitted TAP for a specific week. However, in mint function, only check the number of mint for a single time, not the total number of mint this week, minter can always multiple mint emissionForWeek - 1 to exceed emissionForWeek.

Proof of Concept

    /// @notice returns the amount of emitted TAP for a specific week
    /// @dev week is computed using (timestamp - emissionStartTime) / WEEK
    mapping(uint256 => uint256) public emissionForWeek;

    function extractTAP(address _to, uint256 _amount) external notPaused {
        require(msg.sender == minter, "unauthorized");
        require(_amount > 0, "amount not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        // @audit Here should check `emissionForWeek[week] >= mintedInWeek[week] + amount`
        require(emissionForWeek[week] >= _amount, "exceeds allowable amount");
        _mint(_to, _amount);
        mintedInWeek[week] += _amount;
        emit Minted(msg.sender, _to, _amount);
    }

For specific lines of code, see the comments above. minter can bypass condition checking each time mint emissionForWeek - 1. Another problem is the excess number in the previous week mint, resulting in the subsequent failure to trigger emitForWeek and always revert:

    function emitForWeek() external notPaused returns (uint256) {
        require(_getChainId() == governanceChainIdentifier, "chain not valid");

        uint256 week = _timestampToWeek(block.timestamp);
        if (emissionForWeek[week] > 0) return 0;

        // Update DSO supply from last minted emissions
        dso_supply -= mintedInWeek[week - 1];

        // Compute unclaimed emission from last week and add it to the current week emission
        // @audit if `mintedInWeek > emissionForWeek`, there will be revert
        uint256 unclaimed = emissionForWeek[week - 1] - mintedInWeek[week - 1];
        uint256 emission = uint256(_computeEmission());
        emission += unclaimed;
        emissionForWeek[week] = emission;

        emit Emitted(week, emission);

        return emission;
    }

The user's execution path is: exerciseOption -> _processOTCDeal -> extractTAP, specify mintedInWeek - 1 each time to bypass the restriction and permanently DOS emitForWeek.

Tools Used

Manual review

Recommended Mitigation Steps

should check emissionForWeek[week] >= mintedInWeek[week] + amount instead of emissionForWeek[week] >= _amount.

Assessed type

Context

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph-audit/blob/023751a4e987cf7c203ab25d3abba58f7344f213/contracts/oracle/implementations/ARBTriCryptoOracle.sol#L118

Vulnerability details

Impact

curve classic problem read-only reentrant, can manipulate get_virtual_price to manipulate the ARBTriCryptoOracle.

Proof of Concept

    function _get() internal view returns (uint256 _maxPrice) {
        uint256 _vp = TRI_CRYPTO.get_virtual_price();

        // Get the prices from chainlink and add 10 decimals
        uint256 _btcPrice = uint256(BTC_FEED.latestAnswer()) * 1e10;
        uint256 _wbtcPrice = uint256(WBTC_FEED.latestAnswer()) * 1e10;
        uint256 _ethPrice = uint256(ETH_FEED.latestAnswer()) * 1e10;
        uint256 _usdtPrice = uint256(USDT_FEED.latestAnswer()) * 1e10;

        uint256 _minWbtcPrice = (_wbtcPrice < 1e18)
            ? (_wbtcPrice * _btcPrice) / 1e18
            : _btcPrice;

        uint256 _basePrices = (_minWbtcPrice * _ethPrice * _usdtPrice);

        _maxPrice = (3 * _vp * FixedPointMathLib.cbrt(_basePrices)) / 1 ether;

        // ((A/A0) * (gamma/gamma0)**2) ** (1/3)
        uint256 _g = (TRI_CRYPTO.gamma() * 1 ether) / GAMMA0;
        uint256 _a = (TRI_CRYPTO.A() * 1 ether) / A0;
        uint256 _discount = Math.max((_g ** 2 / 1 ether) * _a, 1e34); // handle qbrt nonconvergence
        // if discount is small, we take an upper bound
        _discount = (FixedPointMathLib.sqrt(_discount) * DISCOUNT0) / 1 ether;

        _maxPrice -= (_maxPrice * _discount) / 1 ether;
    }

As you can see from the code, the contract reads get_virtual_price directly without reentrant protectio.

    D: uint256 = self.get_D(self._balances(), self._A())
    # D is in the units similar to DAI (e.g. converted to precision 1e18)
    # When balanced, D = n * x_u - total virtual value of the portfolio
    # @audit Malicious users can manipulate token_supply through flashloan
    token_supply: uint256 = ERC20(self.lp_token).totalSupply()
    return D * PRECISION / token_supply

The get_virtual_price depended on its balances and its LP token’s total supply. Malicious users can use flashloan to first remove liquidity, increase get_virtual_price, and then re-enter to manipulate ARBTriCryptoOracle.

Tools Used

Manual review

Recommended Mitigation Steps

Use claim_admin_fees to prevent the curve reentrant attack

Assessed type

Reentrancy

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L150 https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/lido/LidoEthStrategy.sol#L162

Vulnerability details

Impact

LidoEthStrategy withdraw usually revert because of toWithdraw miscalculation. There are a couple of issues here:

1. ETH / stETH is not 1:1 on the market. stETH price is higher than ETH, which alleviates the second problem
2. The withdraw has 2.5% slippage, so toWithdraw needs to be 102.5% times. Otherwise, you won't get enough balance later

Proof of Concept

    function _withdraw(
        address to,
        uint256 amount
    ) internal override nonReentrant {
        uint256 available = _currentBalance();
        require(available >= amount, "LidoStrategy: amount not valid");

        uint256 queued = wrappedNative.balanceOf(address(this));
        if (amount > queued) {
            // @audit need multiply 102.5%
            uint256 toWithdraw = amount - queued; //1:1 between eth<>stEth
            uint256 minAmount = toWithdraw - (toWithdraw * 250) / 10_000; //2.5%
            uint256 obtainedEth = curveStEthPool.exchange(
                1,
                0,
                toWithdraw,
                minAmount
            );

            INative(address(wrappedNative)).deposit{value: obtainedEth}();
        }
        queued = wrappedNative.balanceOf(address(this));
        // @audit minAmount = 97.5% toWithdraw and require usually revert
        require(queued >= amount, "LidoStrategy: not enough");

        wrappedNative.safeTransfer(to, amount);

        emit AmountWithdrawn(to, amount);
    }

Actual tests also revert:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";

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

interface ICurveEthStEthPool {
    function exchange(
        int128 i,
        int128 j,
        uint256 dx,
        uint256 min_dy
    ) external payable returns (uint256);

    function get_dy(
        int128 i,
        int128 j,
        uint256 dx
    ) external view returns (uint256);
}

interface INative {
    function deposit() external payable;
}

contract CounterTest is Test {
    address constant wrappedNative = 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2;
    address constant stEth = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;
    address constant curveStEthPool = 0xDC24316b9AE028F1497c275EB9192a3Ea0f67022;

    function setUp() public {
        vm.createSelectFork("https://rpc.ankr.com/eth", 17819438);
    }

    function testCurveSwap() public {
        address me = 0x176F3DAb24a159341c0509bB36B833E7fdd0a132;
        deal(wrappedNative, me, 0);
        vm.startPrank(me);
        IERC20(stEth).approve(curveStEthPool, type(uint256).max);

        uint256 queued = IERC20(wrappedNative).balanceOf(address(this));
        uint256 amount = 1 ether;

        if (amount > queued) {
            uint256 toWithdraw = amount - queued; //1:1 between eth<>stEth
            uint256 minAmount = toWithdraw - (toWithdraw * 250) / 10_000; //2.5%
            uint256 obtainedEth = ICurveEthStEthPool(curveStEthPool).exchange(
                1,
                0,
                toWithdraw,
                minAmount
            );
            INative(wrappedNative).deposit{value: obtainedEth}();
        }
        queued = IERC20(wrappedNative).balanceOf(me);
        vm.expectRevert();
        require(queued >= amount, "LidoStrategy: not enough");
    }
}

Tools Used

Foundry

Recommended Mitigation Steps

Consider that the slippage should be calculated upward: toWithdraw = (amount - queued) * (1000 + 25) / 1000 minAmount = amount - queued

Assessed type

Math