Tapioca DAO - Breeje's results

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L184

Vulnerability details

Impact

Stealing of funds.

Proof of Concept

There is a public function in BaseTOFTLeverageModule contract: leverageDown.

Issue here is there is:

• No Access control (public function).
• No Validation on module address.

  function leverageDown(
        address module,
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) public {
        //-----SNIP: Decode _payload----//

        uint256 balanceBefore = balanceOf(address(this));
        bool credited = creditedPackets[_srcChainId][_srcAddress][_nonce];
        if (!credited) {
            _creditTo(_srcChainId, address(this), amount);
            creditedPackets[_srcChainId][_srcAddress][_nonce] = true;
        }
        uint256 balanceAfter = balanceOf(address(this));

@->     (bool success, bytes memory reason) = module.delegatecall(
            abi.encodeWithSelector(
                this.leverageDownInternal.selector,
                amount,
                swapData,
                externalData,
                lzData,
                leverageFor
            )
        );

        if (!success) {
            if (balanceAfter - balanceBefore >= amount) {
                IERC20(address(this)).safeTransfer(leverageFor, amount);
            }
            revert(_getRevertMsg(reason)); //forward revert because it's handled by the main executor
        }

        emit ReceiveFromChain(_srcChainId, leverageFor, amount);
    }

Link to Code

Leveraging these 2 issues, any attacker can use the following step to exploit the vulnerability:

1. An attacker can deploy their own malicious module contract with a leverageDownInternal function.
2. An attacker can call the leverageDown function, passing their malicious module's address.
3. The malicious leverageDownInternal function is designed to transfer funds from the BaseTOFTLeverageModule contract to the attacker's address.

This way the attacker's module can exploit the unprotected delegatecall to withdraw funds from the BaseTOFTLeverageModule contract without proper authorization.

Tools Used

VS Code

Recommended Mitigation Steps

Have an access control on the function or validate the module address parameter before performing any critical operations.

Assessed type

call/delegatecall

Lines of code

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

Vulnerability details

Impact

Curve Oracle manipulation will allow attacker to wrongfully liquidate users and profit from them.

Proof of Concept

ARBTriCryptoOracle call to get_virtual_price() returns a deflated price when it is reentered after calling the Curve pool's remove_liquidity function. This allows an attacker to liquidate healthy accounts.

get_virtual_price gives the value of an LP token relative to the pool stable asset by dividing the total value of the pool by the totalSupply() of LP tokens:

  @view
  @external
  def get_virtual_price() -> uint256:
      """
      @notice The current virtual price of the pool LP token
      @dev Useful for calculating profits
      @return LP token virtual price normalized to 1e18
      """
      D: uint256 = self.get_D(self._xp(self._stored_rates()), 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
      token_supply: uint256 = ERC20(self.lp_token).totalSupply()
      return D * PRECISION / token_supply

In the Curve pool function remove_liquidity when ETH is withdrawn, raw_call() allows the caller to reenter after the coin balances have been updated, but before the LP tokens are burned, so during the callback a reentrant call to get_virtual_price() will return a deflated value.

  @external
  @nonreentrant('lock')
  def remove_liquidity(_amount: uint256, min_amounts: uint256[N_COINS]) -> uint256[N_COINS]:
      """
      @notice Withdraw coins from the pool
      @dev Withdrawal amounts are based on current deposit ratios
      @param _amount Quantity of LP tokens to burn in the withdrawal
      @param min_amounts Minimum amounts of underlying coins to receive
      @return List of amounts of coins that were withdrawn
      """
      _lp_token: address = self.lp_token
      total_supply: uint256 = ERC20(_lp_token).totalSupply()
      amounts: uint256[N_COINS] = empty(uint256[N_COINS])

      for i in range(N_COINS):
          _balance: uint256 = self.balances[i]
          value: uint256 = _balance * _amount / total_supply
          assert value >= min_amounts[i], "Withdrawal resulted in fewer coins than expected"
          self.balances[i] = _balance - value
          amounts[i] = value
          if i == 0:
              raw_call(msg.sender, b"", value=value)
          else:
              assert ERC20(self.coins[1]).transfer(msg.sender, value)

      CurveToken(_lp_token).burnFrom(msg.sender, _amount)  # Will raise if not enough

      log RemoveLiquidity(msg.sender, amounts, empty(uint256[N_COINS]), total_supply - _amount)

      return amounts

Attacker can use flash loan and reentrancy to carry out this attack. Checkout this References for more details:

This type of vulnerability has been reported here: https://chainsecurity.com/curve-lp-oracle-manipulation-post-mortem/

Post-mortem from MakerDAO here: https://forum.makerdao.com/t/curve-lp-token-oracle-manipulation-vulnerability-technical-postmortem/18009

    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; // @audit-issue Use of 
        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;
    }

Link to code

Tools Used

VS Code

Recommended Mitigation Steps

Add a reentrancy guard to protect against reentrancy.

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

Impact

The liquidator can unfairly liquidate user positions as soon as the protocol is unpaused after a pause. This causes loss of funds to the user.

Proof of Concept

In the BigBang contract, during a paused state, key operations are halted.

The conservator address can pause the BigBang functionalities using the Market.updatePause function. Once the contract is paused all these operations cannot be performed by users:

• borrow
• repay
• addCollateral
• removeCollateral
• liquidate
• buyCollateral
• sellCollateral

It is important to note that during the paused period, actual oracle prices can experience fluctuations. If the oracle prices go down, the users won't be allowed to add more collateral to their positions or close their positions.

Upon unpausing, liquidators can preemptively initiate the liquidation of user positions before they have an opportunity to stabilize their positions.

The absence of a grace period upon unpausing allows liquidators to exploit this window and trigger liquidations immediately, depriving users of the chance to restore their positions.

This vulnerability exposes users to financial loss.

File: BigBang.sol

    function repay(
        address from,
        address to,
        bool,
        uint256 part
@-> ) public notPaused allowedBorrow(from, part) returns (uint256 amount) {
        updateExchangeRate();

        accrue();

        amount = _repay(from, to, part);
    }

    function liquidate(
        address[] calldata users,
        uint256[] calldata maxBorrowParts,
        ISwapper swapper,
        bytes calldata collateralToAssetSwapData
@-> ) external notPaused {

Link to code

Also Sharing a Ref to similar issue reported: Link

Tools Used

VS Code

Recommended Mitigation Steps

Consider implementing a post-unpause grace period during which liquidation remains restricted. This would allow users time to rectify their positions and prevent unfair liquidation.

Assessed type

Timing

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/Penrose.sol#L236

Vulnerability details

Impact

Stealing of all Market Fees.

Proof of Concept

Penrose contract have a public accessed function: withdrawAllMarketFees.

File: Penrose.sol

    function withdrawAllMarketFees(
          IMarket[] calldata markets_,
          ISwapper[] calldata swappers_,
          IPenrose.SwapData[] calldata swapData_
 @->  ) public notPaused {

Link to Code

After input validation, it calls _withdrawAllProtocolFees which handles the main functionality by Looping through the master contracts and call _depositFeesToYieldBox() to each one of their clones.

File: Penrose.sol

    function _depositFeesToYieldBox(
        IMarket market,
        ISwapper swapper,
        IPenrose.SwapData calldata dexData
    ) private {
        //----SNIP: Input Validation-----//

        uint256 feeShares = market.refreshPenroseFees(feeTo);
        if (feeShares == 0) return;

        uint256 assetId = market.assetId();
        uint256 amount = 0;
        if (assetId != wethAssetId) {
            //----SNIP: Transfer asset received from market to swapper-----//
            
            (amount, ) = swapper.swap(
                swapData,
  @->           dexData.minAssetAmount,
                feeTo,
                ""
            );
        } else {
            yieldBox.transfer(address(this), feeTo, assetId, feeShares);
        }

        emit LogYieldBoxFeesDeposit(feeShares, amount);
    }

Here, in case asset is not weth, it uses a swapper to swap the asset which came from market to TAP.

Now consider the following situation:

1. As Anyone can call withdrawAllMarketFees, Alice calls it with a whitelisted swapper and market (to bypass validation) but uses IPenrose.SwapData[] calldata swapData_ = 0 for every market.
2. SwapData contains only one field: minAssetAmount which is the slippage protection.
3. Now with value of dexData.minAssetAmount = 0, the swap done in _depositFeesToYieldBox is vulnerable to Sandwich attack.
4. This allows an attacker Alice to trivially insert transactions before and after this transaction (using the infamous "sandwich" attack), causing the smart contract to trade at a radically worse price, profit from this, and then return the contracts to their original state, all at a low cost.

This way Alice can steal a significant portion of fees, exploiting the transaction at the expense of the fee recipient.

Tools Used

VS Code

Recommended Mitigation Steps

I recommend to either add an access control on withdrawAllMarketFees or add a require check to ensure swapData_ value is atleast above a minimum percentage to prevent potential misuse.

Assessed type

MEV