Tapioca DAO - 0x007's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L442-L462 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/singularity/SGLLiquidation.sol#L303-L314 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L569-L580 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L356-L365 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L427-L480

Vulnerability details

Impact

Liquidators get wrong compensation in reward amount because _getCallerReward uses borrowed part (aka base) to calculate caller reward instead of using borrowed amount (aka elastic). If borrowed base is less than startTVL but the actual borrowed amount is greater than greater than startTVL, the liquidator would not get any reward. When borrowed part is greater than startTVL, reward would be calculated wrongly.

This puts the whole protocol at risk since liquidator get a wrong amount of compensation, which could sometimes be 0.

Proof of Concept

Let's take a look at _getCallerReward

function _getCallerReward(
    uint256 borrowed,
    uint256 startTVLInAsset,
    uint256 maxTVLInAsset
) internal view returns (uint256) {
    if (borrowed == 0) return 0;
    if (startTVLInAsset == 0) return 0;

    if (borrowed < startTVLInAsset) return 0;
    if (borrowed >= maxTVLInAsset) return minLiquidatorReward;

    uint256 rewardPercentage = ((borrowed - startTVLInAsset) *
        FEE_PRECISION) / (maxTVLInAsset - startTVLInAsset);

    int256 diff = int256(minLiquidatorReward) - int256(maxLiquidatorReward);
    int256 reward = (diff * int256(rewardPercentage)) /
        int256(FEE_PRECISION) +
        int256(maxLiquidatorReward);

    return uint256(reward);
}

Because of comparisons and rewardPercentage calculation, it is very important that borrowed, startTVLInAsset and maxTVLInAsset are in the same unit. But they are not.

_getCallerReward is called in Singularity liquidation, Bigbang liquidation and a view function. In all cases, startTVLInAsset and maxTVLInAsset are in elastic amount while borrowed is in base part.

function computeLiquidatorReward(
    address user,
    uint256 _exchangeRate
) public view returns (uint256) {
    (uint256 minTVL, uint256 maxTVL) = _computeMaxAndMinLTVInAsset(
        userCollateralShare[user],
        _exchangeRate
    );
    return _getCallerReward(userBorrowPart[user], minTVL, maxTVL);
}

function _computeMaxAndMinLTVInAsset(
    uint256 collateralShare,
    uint256 _exchangeRate
) internal view returns (uint256 min, uint256 max) {
    uint256 collateralAmount = yieldBox.toAmount(
        collateralId,
        collateralShare,
        false
    );

    max = (collateralAmount * EXCHANGE_RATE_PRECISION) / _exchangeRate;
    min = (max * collateralizationRate) / FEE_PRECISION;
}

Because borrowed amount would not be equal to borrowed part, this would cause liquidator to be compensated with the wrong amount, sometimes 0.

Assuming minLiquidatorReward = 1%, maxLiquidatorReward = 10%, startTVLInAsset = 75, maxTVLInAsset = 100, borrowedAmount = 80, borrowedPart = 74

The liquidator is supposed to get 8.2% because $rewardPercentage = 0.2 = {{80 - 75} \over {100 - 75}}$ therefore $reward = 10 + 0.2(1 - 10)$. But instead they get 0%. Because 74 < 75 when in reality, 80 !< 75.

If the borrowed Part is 75 instead, then liquidator would get max reward which is 10%, higher than the expected 8.2.

If borrowed part is greater than borrowed amount, liquidator would get less reward than expected.

Tools Used

Manual review

Recommended Mitigation Steps

Convert borrowed from base part to elastic amount before rewardPercentage is calculated. This could be done in the _getCallerReward function. It could also be converted before calling _getCallerReward and that way _getCallerReward gets the same unit for all its parameters.

Assessed type

Error

Lines of code

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

Vulnerability details

Impact

A successful manipulation of the oracle would be devastating because it is very important for Markets, OptionBroker and AirdropBroker

Proof of Concept

CurvePool.get_virtual_price can easily be manipulated with read-only reentrancy and the attack is well explained in this article.

During CurvePool.remove_liquidity, ETH or ERC20 tokens are sent out before the Curve token is burn. An attacker can take over the execution flow if the LP contains ETH or the ERC20 has a callback function. At this moment, there's discrepancy between Pool balance (cause token/ETH has been sent), and Pool token supply (cause it has not been burned). CurvePool also has remove_liquidity_imbalance method which can be used to amplify this difference.

@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._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
    token_supply: uint256 = ERC20(self.lp_token).totalSupply()
    return D * PRECISION / token_supply

As can be seen above, D and token_supply would not provide the accurate price because token_supply has not been burned before the reentrancy.

As can be seen below, the returned _maxPrice is directly correlated with TRI_CRYPTO.get_virtual_price(). Therefore a 2x D/token_supply manipulation would result in 2x price manipulation.

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

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;
}

The rough procedure of the attack is:

1. Deposit large amounts of liquidity.
2. Remove liquidity.
3. During the callback perform malicious actions on Market or Options
4. Profit.

Tools Used

Manual review

Recommended Mitigation Steps

This attack is possible because get_virtual_price does not have a reentrancy lock. Find a way to check whether the curve pool is locked or not before returning price. One way to do that is calling non-view function with reentrancy lock such as pool.withdraw_admin_fees or pool.claim_admin_fees.

Assessed type

Oracle

Lines of code

https://github.com/Tapioca-DAO/tapioca-yieldbox-strategies-audit/blob/05ba7108a83c66dada98bc5bc75cf18004f2a49b/contracts/aave/AaveStrategy.sol#L42

Vulnerability details

Impact

Tokens deposited to AAVE Strategy would be locked

Proof of Concept

lendingPool is an address that can only be set once in the constructor and according to AAVE V2 docs

Whenever the LendingPool contract is needed, we recommended you fetch the correct address from the LendingPoolAddressesProvider smart contract.

A better explanation for why is given in the V1 docs

The smart contracts in Aave Protocol are upgradeable through governance. The protocol keeps a global register of the latest addresses deployed on a particular network (i.e. Mainnet, Kovan, Ropsten).

If the lendingPool is upgraded and the address does change, then AAVEStrategy would not work and funds would not be retrievable.

Tools Used

Manual review

Recommended Mitigation Steps

Follow AAVE's recommendation and use LendingPoolAddressesProvider.getLendingPool() to retrieve the latest pool address before calling it.

Assessed type

Upgradable

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L52 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L84-L91 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L99-L102 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L261-L275 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L247-L253 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L281-L281 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L295-L300 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L373-L374 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L393-L394

Vulnerability details

Impact

allowanceBorrow is meant for collateral shares but asset share is deducted in BigBang.repay. This would lead to a situation where user could loose asset instead of collateral. And if the asset is more valuable than collateral, then user would lose more USD value than they expected.

Proof of Concept

allowanceBorrow is used by users to allow other addresses to use some of their deposited collateral via _allowedBorrow function or allowedBorrow modifier. allowanceBorrow is used in borrow, addCollateral, removeCollateral, buyCollateral and sellCollateral. In all this case, amount that's added or deducted from allowanceBorrow is collateral share. Infact, borrow accepts asset amount as collateral but converted it to collateral share with _computeAllowanceAmountInAsset.

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

/// @notice Adds `collateral` from msg.sender to the account `to`.
/// @param from Account to transfer shares from.
/// @param to The receiver of the tokens.
/// @param skim True if the amount should be skimmed from the deposit balance of msg.sender.
/// False if tokens from msg.sender in `yieldBox` should be transferred.
/// @param share The amount of shares to add for `to`.
function addCollateral(
    address from,
    address to,
    bool skim,
    uint256 amount,
    uint256 share
) public allowedBorrow(from, share) notPaused {
    _addCollateral(from, to, skim, amount, share);
}

/// @notice Removes `share` amount of collateral and transfers it to `to`.
/// @param from Account to debit collateral from.
/// @param to The receiver of the shares.
/// @param share Amount of shares to remove.
function removeCollateral(
    address from,
    address to,
    uint256 share
) public notPaused solvent(from) allowedBorrow(from, share) {
    _removeCollateral(from, to, share);
}

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

/// @param amount Amount to borrow.
/// @return part Total part of the debt held by borrowers.
/// @return share Total amount in shares borrowed.
function borrow(
    address from,
    address to,
    uint256 amount
) public notPaused solvent(from) returns (uint256 part, uint256 share) {
    uint256 allowanceShare = _computeAllowanceAmountInAsset(
        from,
        exchangeRate,
        amount,
        asset.safeDecimals()
    );
    _allowedBorrow(from, allowanceShare);
    (part, share) = _borrow(from, to, amount);
}

But there's an exception and that's the repay function. Where the deducted amount is asset share.

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

/// @param part The amount to repay. See `userBorrowPart`.
/// @return amount The total amount repayed.
function repay(
    address from,
    address to,
    bool,
    uint256 part
) public notPaused allowedBorrow(from, part) returns (uint256 amount) {
    updateExchangeRate();

    accrue();

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

In BigBang, the asset is USD0 which is pegged to USD. Assuming the collateral is worth less than 1 USD such as GRT which is worth approximately 0.1 USD. We'll also assume that 1 share = 1 amount for both collateral and asset for simplicity.

Let's say Alice has 200,000 GRT collateral, 5,000 USD0 asset and provided allowanceBorrow of 1,000 GRT to another address Bob. And Alice expectation is that Bob takes the 1,000 GRT which is worth 100 USD0 and add it as collateral for Bob's account.

But another thing that Bob could do is repay 1,000 USD0 of Bob's debt. Therefore, Alice would lose 1,000 USD instead of 100 USD which is 10x.

Also, the reverse scenario is possible whereby user thinks they are providing allowance for repay asset, but lose collateral which might be valuable when Bob calls addCollateral instead.

Tools Used

Manual review

Recommended Mitigation Steps

Convert the asset share to collateral share. The collateral share should then be deducted from allowanceBorrow instead of the asset share.

Assessed type

Math

Lines of code

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

Vulnerability details

Impact

If there's bad debt, USD0 would depeg to 0.

Proof of Concept

The focus is on _updateBorrowAndCollateralShare which is called from _liquidateUser. It is possible for userCollateralShare to be 0 while userBorrowPart is greater than 0. This means that the user has debt but no collateral. Such a position cannot be liquidated traditionally because you need collateral share to swap for asset that would be repaid.

The 3 likely ways for bad debt to accumulate are

1. Great price fall such as stablecoin depegs, price crash like LUNC. This would be made worse cause oracles sometimes lag and liquidation works in such a way that it liquidates an amount that's just enough for the liquidated user to become solvent again. Gas prices also tend to jump through the roof during these times.
2. Fall in Strategy amounts because of events like hacks. Vyper 0 day :(
3. Since there's no cap on user borrow, there might not be enough liquidity to quickly swap users collateral at a good price.

When there's bad debt, USD0 would become under-collateralized, and this breaks a major property of the protocol. Once users notice this they would want to redeem USD0 for their collateral and start selling. This would result in case where the price could fall to almost 0. If all users with collateral redeem their USD0 for collateral, then non of the circulating supply would be backed by collateral.

Tools Used

Manual Review

Recommended Mitigation Steps

Create a way to liquidate bad debts

Assessed type

Other