Tapioca DAO - RedOneN's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L282-L290 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L543-L558 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L99-L101 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/MarketERC20.sol#L84-L91

Vulnerability details

Impact

The vulnerability resides in the 'addCollateral()' function of the BigBang contract. Looking at the function definition:

  function addCollateral(
     address from,
     address to,
     bool skim,
     uint256 amount,
     uint256 share
  ) public allowedBorrow(from, share) notPaused {

we see an 'allowedBorrow()' modifier that checks if the spender (msg.sender) is authorized to spend a value of ‘share’ from an account ‘from’. This check can be bypassed by providing a 0 value for 'share' and a non-zero value for 'amount'. We then reach the following part of the function :

    if (share == 0) {
        share = yieldBox.toShare(collateralId, amount, false);          
    }
    userCollateralShare[to] += share;
    uint256 oldTotalCollateralShare = totalCollateralShare;
    totalCollateralShare = oldTotalCollateralShare + share;  

By setting 'share' to zero, the protocol will compute a 'share' value based on the 'amount' within the function. The contract then increments the spender's share balance (userCollateralShare[to]) without checking the allowance requirement, essentially allowing any user to bypass the approval check and steal collateral shares from another user.

This means that an attacker can steal collateral shares from any user by exploiting this vulnerability, causing significant asset loss to the affected users and potentially disrupting the entire protocol.

Proof of Concept

Consider the following scenario:

 contractInstance.addCollateral(fromAddress, toAddress, false, 1000000, 0);

In the above example, an attacker calls the 'addCollateral()' function, specifying 'fromAddress' as the victim's address and 'toAddress' as the attacker's own address. The 'share' is passed as 0, and 'amount' is passed as 1000000. As a result, the 'allowedBorrow()' check is bypassed, and 'amount' is converted to 'share' within the function. Consequently, 1000000 units are transferred from the victim's account to the attacker's account without the proper authorization check.

Tools Used

Manual Audit

Recommended Mitigation Steps

To prevent such a bypass, it is recommended to check the allowance after the conversion of ‘amount’ to ‘share’ :

 function _addCollateral(
     address from,
     address to,
     bool skim,
     uint256 amount,
     uint256 share
 ) internal {
     if (share == 0) {
         share = yieldBox.toShare(collateralId, amount, false);
     }
   _allowedBorrow(from, share); 
    ...
 }

Note that the ‘allowedBorrow(from, share)’ modifier can now be removed.

Assessed type

Context

Lines of code

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

Vulnerability details

Impact

Within the getDebtRate() function of the contract, a vulnerability exists that could potentially hinder the operation of the contract. This vulnerability arises when _currentDebt falls below debtStartPoint, triggering an overflow in the computation of debtPercentage. As the getDebtRate() function is indirectly called in each important function (through ‘_accrue()’) such as borrow(), repay(), removeCollateral(), and liquidate(), this overflow issue could break the functionality of the contract.

This issue is particularly impactful under the following scenarios:

1. At the beginning of the contract deployment when _currentDebt is low due to lack of borrowing.
2. When significant liquidations occur, reducing _currentDebt drastically.
3. When a user chooses to repay a position that brings the total borrow amount below debtStartPoint.

Note that the last point is a way for a malicious user to manipulate and break the protocol.

Proof of Concept

The proof of concept is embedded in the contract code. In the getDebtRate() function, the calculation for debtPercentage is executed as follows:

 uint256 debtPercentage = ((_currentDebt - debtStartPoint) * DEBT_PRECISION) / (_maxDebtPoint - debtStartPoint);     
            

However, no condition exists that checks whether _currentDebt is greater than or equal to debtStartPoint. If _currentDebt falls below debtStartPoint, this subtraction will trigger an underflow, causing the contract to revert. This prevents the getDebtRate() function from returning a valid "protocol debt rate", which could cause the contract to revert on most of the contract’s function.

Tools Used

Manual Audit

Recommended Mitigation Steps

To prevent this issue, a check should be added before the computation of debtPercentage:

 if (debtStartPoint >= _currentDebt) return 0;

This check ensures that if _currentDebt is less than debtStartPoint, the function returns 0, preventing a potential underflow and maintaining the operation of the contract.

Assessed type

Under/Overflow

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/usd0/USDO.sol#L81-L104

Vulnerability details

Impact

The ‘flashLoan()’ function inside the USDO contract prevent users from performing a excessively large flash mint through the following condition :

 require(maxFlashLoan(token) >= amount, "USDO: amount too big");

However, the ‘receiver.onFlashLoan()’ external call observed latter in the function opens the door for a potential reentrancy. Indeed, if the receiver contract calls back the ‘onFlashLoan()’ multiple times with an amount lower or equals to ‘maxFlashLoan’, he will be able to bypassed the condition and borrow an amount way larger than the maximum value defined within the same ‘transaction’ and avoid the rule set by the protocol.

Proof of Concept

The flashLoan function in the provided Solidity contract allows for flash loans, but contains a reentrancy vulnerability. Here is the related code snippet:

 require(maxFlashLoan(token) >= amount, "USDO: amount too big");                           
 require(amount > 0, "USDO: amount not valid");
 uint256 fee = flashFee(token, amount);
  _mint(address(receiver), amount);

  require(receiver.onFlashLoan(msg.sender, token, amount, fee, data) ==  FLASH_MINT_CALLBACK_SUCCESS, "USDO: failed");

During the execution of flashLoan, the contract verifies if the amount to be borrowed is less than or equal to maxFlashLoan. If it is, the contract mints the amount for the receiver. It then calls onFlashLoan on the receiver, and it's during this call that the vulnerability can be exploited.

If the receiver is a malicious contract, it can reenter the flashLoan function multiple times. Each reentry would check the maxFlashLoan constraint, but the amount for each reentry could be designed such that it is less than maxFlashLoan. This means multiple reentries can occur within a single transaction, each minting tokens for the malicious receiver. In the end, this allows the malicious contract to mint much more tokens than the intended maxFlashLoan limit.

Tools Used

Manual Audit

Recommended Mitigation Steps

Use a reentrancy guard for the flashloan() function:

 bool internal locked;

 modifier noReentrant() {
    require(!locked, "No re-entrancy");
    locked = true;
    _;
    locked = false;
 }

Assessed type

Reentrancy

Lines of code

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

Vulnerability details

The BigBang contract has an internal accounting mechanism that keeps track of the collateral share of each user (ie userCollateralShare) and the total amount of collateral supplied (ie totalCollateralShare).

Consequently, each time that a user add collateral, these two variables are updates as we can is in the _addCollateral() function :

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

    userCollateralShare[to] += share;
    uint256 oldTotalCollateralShare = totalCollateralShare;
    totalCollateralShare = oldTotalCollateralShare + share;  

When a user removes collateral, these variables are obviously updated as well as we can see in the _removeCollateral() function :

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

    userCollateralShare[from] -= share;
    totalCollateralShare -= share;

As a result, we expect the liquidation function to update these two variables as well since a liquidation is equivalent to selling/removing collateral from the protocol. However, when we look at the liquidation function, more specifically at the sub-function _updateBorrowAndCollateralShare() being called during the liquidation :

function _updateBorrowAndCollateralShare(
    address user,
    uint256 maxBorrowPart,
    uint256 _exchangeRate
)
    private
    returns (
        uint256 borrowAmount,
        uint256 borrowPart,
        uint256 collateralShare
    )
{
    uint256 collateralPartInAsset = (yieldBox.toAmount(                 
        collateralId,
        userCollateralShare[user],
        false
    ) * EXCHANGE_RATE_PRECISION) / _exchangeRate;

    uint256 borrowAssetDecimals = asset.safeDecimals();                 
    uint256 collateralDecimals = collateral.safeDecimals();

    uint256 availableBorrowPart = computeClosingFactor(                 
        userBorrowPart[user],
        collateralPartInAsset,
        borrowAssetDecimals,
        collateralDecimals,
        FEE_PRECISION_DECIMALS
    );
    borrowPart = maxBorrowPart > availableBorrowPart               
        ? availableBorrowPart
        : maxBorrowPart;

    if (borrowPart > userBorrowPart[user]) {                       
    }

    userBorrowPart[user] = userBorrowPart[user] - borrowPart;      

    borrowAmount = totalBorrow.toElastic(borrowPart, false);
    uint256 amountWithBonus = borrowAmount +
        (borrowAmount * liquidationMultiplier) /
        FEE_PRECISION;
    collateralShare = yieldBox.toShare(
        collateralId,
        (amountWithBonus * _exchangeRate) / EXCHANGE_RATE_PRECISION,
        false
    );
    if (collateralShare > userCollateralShare[user]) {            
        collateralShare = userCollateralShare[user];
    }
    userCollateralShare[user] -= collateralShare;
    require(borrowAmount != 0, "SGL: solvent");

    totalBorrow.elastic -= uint128(borrowAmount);
    totalBorrow.base -= uint128(borrowPart);
}

}

We can see that userCollateralShare is correctly updated : ‘userCollateralShare[user] -= collateralShare;’ but the ‘totalCollateralShare’ is not updated at all.This results in an incorrect accounting monitoring system inside the protocol.

Proof Of Concept

See description above.

Tools used

Manual Audit

Recommended Mitigation Steps

Add the following line right after updating the userCollateralShare :

totalCollateralShare-= collateralShare;

Assessed type

Context