Tapioca DAO - Koolex's results

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L402-L425

Vulnerability details

Impact

When a Collateralized Debt Position (CDP) reaches that liquidation threshold, it becomes eligible for liquidation and anyone can repay a position in exchange for a portion of the collateral. Market._isSolvent is used to check if the user is solvent. if not, then it can be liquidated. Here is the method body:

function _isSolvent(
	address user,
	uint256 _exchangeRate
) internal view returns (bool) {
	// accrue must have already been called!
	uint256 borrowPart = userBorrowPart[user];
	if (borrowPart == 0) return true;
	uint256 collateralShare = userCollateralShare[user];
	if (collateralShare == 0) return false;

	Rebase memory _totalBorrow = totalBorrow;

	return
		yieldBox.toAmount(
			collateralId,
			collateralShare *
				(EXCHANGE_RATE_PRECISION / FEE_PRECISION) *
				collateralizationRate,
			false
		) >=
		// Moved exchangeRate here instead of dividing the other side to preserve more precision
		(borrowPart * _totalBorrow.elastic * _exchangeRate) /
			_totalBorrow.base;
}

Code link

The issue is that the collateralizationRate is multiplied by collateralShare (with precision constants) then converted to amount. This is incorrect, the collateralizationRate sholud be used with amounts and not shares. Otherwise, we get wrong results.

yieldBox.toAmount(
			collateralId,
			collateralShare *
				(EXCHANGE_RATE_PRECISION / FEE_PRECISION) *
				collateralizationRate,
			false
		)

Please note that when using shares it is not in favour of the protocol, so amounts should be used instead. The only case where this is ok, is when the share/amount ratio is 1:1 which can not be, because totalAmount always get +1 and totalShares +1e8 to prevent 1:1 ratio type of attack.

    function _toAmount(
        uint256 share,
        uint256 totalShares_,
        uint256 totalAmount,
        bool roundUp
    ) internal pure returns (uint256 amount) {
        // To prevent reseting the ratio due to withdrawal of all shares, we start with
        // 1 amount/1e8 shares already burned. This also starts with a 1 : 1e8 ratio which
        // functions like 8 decimal fixed point math. This prevents ratio attacks or inaccuracy
        // due to 'gifting' or rebasing tokens. (Up to a certain degree)
        totalAmount++;
        totalShares_ += 1e8;

Code link

Moreover, in the method _computeMaxAndMinLTVInAsset which is supposed to returns the min and max LTV for user in asset price. Amount is used and not share. Here is the code:

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

Code Link

I've set this to high severity because solvency check is a crucial part of the protocol. In short, we have :

1. Inconsistency across the protocol
2. Inaccuracy of calculating the liquidation threshold
3. Not in favour of the protocol

Note: this is also applicable for ohter methods. For example, Market._computeMaxBorrowableAmount.

Proof of Concept

• When you run the PoC below, you will get the following results:

[PASS] test_borrow_repay() (gas: 118001)
Logs:
  ===BORROW===
  UserBorrowPart: 745372500000000000000
  Total Borrow Base: 745372500000000000000
  Total Borrow Elastic: 745372500000000000000
  ===356 days passed===
  Total Borrow Elastic: 749089151896269477984
  ===Solvency#1 => multiply by share===
  A: 749999999999925000000750007499999924999
  B: 749089151896269477984000000000000000000
  ===Solvency#2 => multiply by amount===
  A: 749999999999925000000750000000000000000
  B: 749089151896269477984000000000000000000
  ===Result===
  Solvency#1.A != Solvency#2.A

Test result: ok. 1 passed; 0 failed; finished in 16.69ms

As you can see, numbers are not equal, and when using shares it is not in favour of the protocol, so amount should be used instead.

• Code: Please note some lines in borrow method were commented out for simplicity. It is irrelevant anyway.
• _toAmount copied from YieldBoxRebase

// PoC => BIGBANG - Solvency Check Inaccuracy
// Command => forge test -vv
pragma solidity >=0.8.4 <0.9.0;

import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

import {DSTest} from "ds-test/test.sol";
struct AccrueInfo {
    uint64 debtRate;
    uint64 lastAccrued;
}

struct Rebase {
    uint128 elastic;
    uint128 base;
}

/// @notice A rebasing library using overflow-/underflow-safe math.
library RebaseLibrary {
    /// @notice Calculates the base value in relationship to `elastic` and `total`.
    function toBase(
        Rebase memory total,
        uint256 elastic,
        bool roundUp
    ) internal pure returns (uint256 base) {
        if (total.elastic == 0) {
            base = elastic;
        } else {
            base = (elastic * total.base) / total.elastic;
            if (roundUp && (base * total.elastic) / total.base < elastic) {
                base++;
            }
        }
    }

    /// @notice Calculates the elastic value in relationship to `base` and `total`.
    function toElastic(
        Rebase memory total,
        uint256 base,
        bool roundUp
    ) internal pure returns (uint256 elastic) {
        if (total.base == 0) {
            elastic = base;
        } else {
            elastic = (base * total.elastic) / total.base;
            if (roundUp && (elastic * total.base) / total.elastic < base) {
                elastic++;
            }
        }
    }

    /// @notice Add `elastic` to `total` and doubles `total.base`.
    /// @return (Rebase) The new total.
    /// @return base in relationship to `elastic`.
    function add(
        Rebase memory total,
        uint256 elastic,
        bool roundUp
    ) internal pure returns (Rebase memory, uint256 base) {
        base = toBase(total, elastic, roundUp);
        total.elastic += uint128(elastic);
        total.base += uint128(base);
        return (total, base);
    }

    /// @notice Sub `base` from `total` and update `total.elastic`.
    /// @return (Rebase) The new total.
    /// @return elastic in relationship to `base`.
    function sub(
        Rebase memory total,
        uint256 base,
        bool roundUp
    ) internal pure returns (Rebase memory, uint256 elastic) {
        elastic = toElastic(total, base, roundUp);
        total.elastic -= uint128(elastic);
        total.base -= uint128(base);
        return (total, elastic);
    }

    /// @notice Add `elastic` and `base` to `total`.
    function add(
        Rebase memory total,
        uint256 elastic,
        uint256 base
    ) internal pure returns (Rebase memory) {
        total.elastic += uint128(elastic);
        total.base += uint128(base);
        return total;
    }

    /// @notice Subtract `elastic` and `base` to `total`.
    function sub(
        Rebase memory total,
        uint256 elastic,
        uint256 base
    ) internal pure returns (Rebase memory) {
        total.elastic -= uint128(elastic);
        total.base -= uint128(base);
        return total;
    }

    /// @notice Add `elastic` to `total` and update storage.
    /// @return newElastic Returns updated `elastic`.
    function addElastic(
        Rebase storage total,
        uint256 elastic
    ) internal returns (uint256 newElastic) {
        newElastic = total.elastic += uint128(elastic);
    }

    /// @notice Subtract `elastic` from `total` and update storage.
    /// @return newElastic Returns updated `elastic`.
    function subElastic(
        Rebase storage total,
        uint256 elastic
    ) internal returns (uint256 newElastic) {
        newElastic = total.elastic -= uint128(elastic);
    }
}

contract BIGBANG_MOCK {
    using RebaseLibrary for Rebase;
    uint256 public collateralizationRate = 75000; // 75% // made public to access it from test contract
    uint256 public liquidationMultiplier = 12000; //12%
    uint256 public constant FEE_PRECISION = 1e5; // made public to access it from test contract
    uint256 public EXCHANGE_RATE_PRECISION = 1e18; //made public to access it from test contract

    uint256 public borrowOpeningFee = 50; //0.05%
    Rebase public totalBorrow;
    uint256 public totalBorrowCap;
    AccrueInfo public accrueInfo;

    /// @notice borrow amount per user
    mapping(address => uint256) public userBorrowPart;

    uint256 public USDO_balance; // just to track USDO balance of BigBang

    function _accrue() public {
        // made public so we can call it from the test contract
        AccrueInfo memory _accrueInfo = accrueInfo;
        // Number of seconds since accrue was called
        uint256 elapsedTime = block.timestamp - _accrueInfo.lastAccrued;
        if (elapsedTime == 0) {
            return;
        }
        //update debt rate // for simplicity we return bigBangEthDebtRate which is 5e15
        uint256 annumDebtRate = 5e15; // getDebtRate(); // 5e15 for eth. Check Penrose.sol Line:131
        _accrueInfo.debtRate = uint64(annumDebtRate / 31536000); //per second

        _accrueInfo.lastAccrued = uint64(block.timestamp);

        Rebase memory _totalBorrow = totalBorrow;

        uint256 extraAmount = 0;

        // Calculate fees
        extraAmount =
            (uint256(_totalBorrow.elastic) *
                _accrueInfo.debtRate *
                elapsedTime) /
            1e18;
        _totalBorrow.elastic += uint128(extraAmount);

        totalBorrow = _totalBorrow;
        accrueInfo = _accrueInfo;

        // emit LogAccrue(extraAmount, _accrueInfo.debtRate); // commented out since it irrelevant
    }

    function totalBorrowElastic() public view returns (uint128) {
        return totalBorrow.elastic;
    }

    function totalBorrowBase() public view returns (uint128) {
        return totalBorrow.base;
    }

    function _borrow(
        address from,
        address to,
        uint256 amount
    ) external returns (uint256 part, uint256 share) {
        uint256 feeAmount = (amount * borrowOpeningFee) / FEE_PRECISION; // A flat % fee is charged for any borrow

        (totalBorrow, part) = totalBorrow.add(amount + feeAmount, true);
        require(
            totalBorrowCap == 0 || totalBorrow.elastic <= totalBorrowCap,
            "BigBang: borrow cap reached"
        );

        userBorrowPart[from] += part; // toBase from RebaseLibrary. userBorrowPart stores the sharee

        //mint USDO
        // IUSDOBase(address(asset)).mint(address(this), amount); // not needed
        USDO_balance += amount;

        //deposit borrowed amount to user
        // asset.approve(address(yieldBox), amount);  // not needed
        // yieldBox.depositAsset(assetId, address(this), to, amount, 0); // not needed
        USDO_balance -= amount;

        // share = yieldBox.toShare(assetId, amount, false); // not needed

        // emit LogBorrow(from, to, amount, feeAmount, part); // not needed
    }

    // copied from YieldBoxRebase
    function _toAmount(
        uint256 share,
        uint256 totalShares_,
        uint256 totalAmount,
        bool roundUp
    ) external pure returns (uint256 amount) {
        // To prevent reseting the ratio due to withdrawal of all shares, we start with
        // 1 amount/1e8 shares already burned. This also starts with a 1 : 1e8 ratio which
        // functions like 8 decimal fixed point math. This prevents ratio attacks or inaccuracy
        // due to 'gifting' or rebasing tokens. (Up to a certain degree)
        totalAmount++;
        totalShares_ += 1e8;

        // Calculte the amount using te current amount to share ratio
        amount = (share * totalAmount) / totalShares_;

        // Default is to round down (Solidity), round up if required
        if (roundUp && (amount * totalShares_) / totalAmount < share) {
            amount++;
        }
    }
}

contract BIGBANG_ISSUES is DSTest, Test {
    BIGBANG_MOCK bigbangMock;
    address bob;

    function setUp() public {
        bigbangMock = new BIGBANG_MOCK();
        bob = vm.addr(1);
    }

    function test_borrow_repay() public {
        // borrow
        uint256 amount = 745e18;

        vm.warp(1 days);
        bigbangMock._accrue(); // acrrue before borrow (this is done on borrow)

        bigbangMock._borrow(bob, address(0), amount);
        console.log("===BORROW===");
        // console.log("Amount: %d", amount);
        console.log("UserBorrowPart: %d", bigbangMock.userBorrowPart(bob));
        console.log("Total Borrow Base: %d", bigbangMock.totalBorrowBase());
        console.log(
            "Total Borrow Elastic: %d",
            bigbangMock.totalBorrowElastic()
        );

        // time elapsed
        vm.warp(365 days);
        console.log("===356 days passed===");
        bigbangMock._accrue();
        console.log(
            "Total Borrow Elastic: %d",
            bigbangMock.totalBorrowElastic()
        ); 

        // Check Insolvency

        uint256 _exchangeRate = 1e18;
        uint256 collateralShare = 1000e18;
        uint256 totalShares = 1000e18;
        uint256 totalAmount = 1000e18;
        uint256 EXCHANGE_RATE_PRECISION = bigbangMock.EXCHANGE_RATE_PRECISION();
        uint256 FEE_PRECISION = bigbangMock.FEE_PRECISION();
        uint256 collateralizationRate = bigbangMock.collateralizationRate();

        uint256 borrowPart = bigbangMock.userBorrowPart(bob);
        uint256 _totalBorrowElastic = bigbangMock.totalBorrowElastic();
        uint256 _totalBorrowBase = bigbangMock.totalBorrowBase();



        console.log("===Solvency#1 => multiply by share===");
        // we pass totalShares and totalAmount
        uint256 A = bigbangMock._toAmount(
            collateralShare *
                (EXCHANGE_RATE_PRECISION / FEE_PRECISION) *
                collateralizationRate,
            totalShares,
            totalAmount,
            false
        );

        // Moved exchangeRate here instead of dividing the other side to preserve more precision
        uint256 B = (borrowPart * _totalBorrowElastic * _exchangeRate) /
            _totalBorrowBase;

        // bool isSolvent = A >= B;

        console.log("A: %d", A);
        console.log("B: %d", B);


        console.log("===Solvency#2 => multiply by amount===");

        A =
            bigbangMock._toAmount(
                collateralShare,
                totalShares,
                totalAmount,
                false
            ) *
            (EXCHANGE_RATE_PRECISION / FEE_PRECISION) *
            collateralizationRate;

        // Moved exchangeRate here instead of dividing the other side to preserve more precision
        B =
            (borrowPart * _totalBorrowElastic * _exchangeRate) /
            _totalBorrowBase;

        // isSolvent = A >= B;

        console.log("A: %d", A);
        console.log("B: %d", B);


        console.log("===Result===");
        console.log("Solvency#1.A != Solvency#2.A");

    }
}

Tools Used

Manual analysis

Recommended Mitigation Steps

Use amount for calculation instead of shares. Check the PoC as it demonstrates such an example.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

addCollateral allows anyone to addCollateral on behalf of others. In other words, bypassing the borrow allowance check.

Proof of Concept

allowedBorrow modifier will not revert if passed share == 0. addCollateral() method uses allowedBorrow modifier

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

Code link

Now let's have a look at the modifier. It basically calls _allowedBorrow()

    modifier allowedBorrow(address from, uint share) virtual {
        _allowedBorrow(from, share);
        _;
    }

The method _allowedBorrow() is aware only of the share and not aware of the amount passed. If the share was passed as zero, then it bypass the modifier.

    function _allowedBorrow(address from, uint share) internal {
        if (from != msg.sender) {
            if (allowanceBorrow[from][msg.sender] < share) {
                revert NotApproved(from, msg.sender);
            }
            allowanceBorrow[from][msg.sender] -= share;
        }
    }

Code link

Then in _addCollateral() , share is calculated (since it was passed zero) based on the amount

	if (share == 0) {
		share = yieldBox.toShare(collateralId, amount, false);
	}

Code link

This calculation happen afte bypassing the modifier. Which means anyone can bypass the allowance validation for anyone.

Tools Used

Manual Review

Recommended Mitigation Steps

Use _allowedBorrow method instead of the modifier. Call the method after the share calculation.

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

When calling _addCollateral, and if the share passed as zero, it is calculated based on the passed amount. However, this happens after allowanceBorrow was already called in addCollateral. So, deduction never occur for the share. Eventually, the borrow allowance never decreases. This allows a user (who has borrow allowance) to call addCollateral without borrow allowance limit set by the owner.

Proof of Concept

No deduction for the share allowance from _addCollateral() for from address:

    function _addCollateral(
        address from,
        address to,
        bool skim,
        uint256 amount,
        uint256 share
    ) internal {
        if (share == 0) {
            share = yieldBox.toShare(collateralId, amount, false);
        }
        userCollateralShare[to] += share;
        uint256 oldTotalCollateralShare = totalCollateralShare;
        totalCollateralShare = oldTotalCollateralShare + share;
        _addTokens(from, collateralId, share, oldTotalCollateralShare, skim);
        emit LogAddCollateral(skim ? address(yieldBox) : from, to, share);
    }

Code link

The share is calculated here:

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

but there is no decrease for the share from allowanceBorrow map afterwards (incase the amount > 0 and the share is 0).
We know it is already done in allowedBorrow modifier in _allowedBorrow() method but since the share was passed zero, it had no effect.

 allowanceBorrow[from][msg.sender] -= share;

Code link

Tools Used

Manual Review

Recommended Mitigation Steps

decrease the allowanceBorrow for the share after being calucalted in _addCollateral() method in case the share passed is zero.

//add this line in _addCollateral() after share being calculated only if share was passed zero.
 allowanceBorrow[from][msg.sender] -= share;

Assessed type

Other

Lines of code

https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/bigBang/BigBang.sol#L790-L796 https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/2286f80f928f41c8bc189d0657d74ba83286c668/contracts/markets/Market.sol#L290-L293

Vulnerability details

Impact

computeClosingFactor method returns the maximum liquidatable amount for user. In BigBang, this method is called by _updateBorrowAndCollateralShare which is basically called by other methods on liquidation as follows: liquidate => _closedLiquidation => _liquidateUser => _updateBorrowAndCollateralShare => computeClosingFactor

Here is code of _updateBorrowAndCollateralShare method where it calls computeClosingFactor

	function _updateBorrowAndCollateralShare(
		address user,
		uint256 maxBorrowPart,
		uint256 _exchangeRate
	)
	.
	.
	uint256 availableBorrowPart = computeClosingFactor(
		userBorrowPart[user],
		collateralPartInAsset,
		borrowAssetDecimals,
		collateralDecimals,
		FEE_PRECISION_DECIMALS
	);
	.
	.

Code link

The issue is computeClosingFactor will revert if collateralizationRate is greater than 89285. The revert is due to Arithmetic over/underflow or Division by 0 (depending on how higher the collateralizationRate is). This causes callers (methods of liquidations) to fail as well. Eventually, blocking liquidation functionality completely which is obviously harmful to the protocol and any party that benefit from it (e.g. liquidators).

Here is an explanation how this is caused: The denominator is calculated as follows

        uint256 denominator = ((10 ** ratesPrecision) -
            (collateralizationRate *
                ((10 ** ratesPrecision) + liquidationMultiplier)) /
            (10 ** ratesPrecision)) * (10 ** (18 - ratesPrecision));

Code link

For simplicity, assume B = (collateralizationRate * ((10 ** ratesPrecision) + liquidationMultiplier)) A = (10 ** ratesPrecision)

Now, B is deducted from A. if B > A, then the method will revert.
if A == B, denominator will be zero. therefore, a division by zero occurs later.

Please note that this is also applicable to other markets (e.g. Singularity) since they inherit from Market contract. Check SGLLiquidation._updateBorrowAndCollateralShare Line: 225

Proof of Concept

• When you run the PoC below, you will get the following results:

[PASS] test_colRate_greater_than_89286_will_revert() (gas: 34473)
Logs:
  collateralizationRate 89286: revert with Division or modulo by 0
  collateralizationRate 9000: revert with Arithmetic over/underflow

[PASS] test_colRate_lower_than_89286_works() (gas: 31540)
Logs:
  collateralizationRate 75000: works as intended
  collateralizationRate 89285: works as intended

Test result: ok. 2 passed; 0 failed; finished in 1.24ms

The results and the code are self-explanatory.

• Code

// PoC => BIGBANG  => Closing Factor Revert
// Command => forge test -vv
pragma solidity >=0.8.4 <0.9.0;

import {Test} from "forge-std/Test.sol";
import "forge-std/console.sol";

import {DSTest} from "ds-test/test.sol";

struct Rebase {
    uint128 elastic;
    uint128 base;
}

/// @notice A rebasing library using overflow-/underflow-safe math.
library RebaseLibrary {
    /// @notice Calculates the base value in relationship to `elastic` and `total`.
    function toBase(
        Rebase memory total,
        uint256 elastic,
        bool roundUp
    ) internal pure returns (uint256 base) {
        if (total.elastic == 0) {
            base = elastic;
        } else {
            base = (elastic * total.base) / total.elastic;
            if (roundUp && (base * total.elastic) / total.base < elastic) {
                base++;
            }
        }
    }

    /// @notice Calculates the elastic value in relationship to `base` and `total`.
    function toElastic(
        Rebase memory total,
        uint256 base,
        bool roundUp
    ) internal pure returns (uint256 elastic) {
        if (total.base == 0) {
            elastic = base;
        } else {
            elastic = (base * total.elastic) / total.base;
            if (roundUp && (elastic * total.base) / total.elastic < base) {
                elastic++;
            }
        }
    }

    /// @notice Add `elastic` to `total` and doubles `total.base`.
    /// @return (Rebase) The new total.
    /// @return base in relationship to `elastic`.
    function add(
        Rebase memory total,
        uint256 elastic,
        bool roundUp
    ) internal pure returns (Rebase memory, uint256 base) {
        base = toBase(total, elastic, roundUp);
        total.elastic += uint128(elastic);
        total.base += uint128(base);
        return (total, base);
    }

    /// @notice Sub `base` from `total` and update `total.elastic`.
    /// @return (Rebase) The new total.
    /// @return elastic in relationship to `base`.
    function sub(
        Rebase memory total,
        uint256 base,
        bool roundUp
    ) internal pure returns (Rebase memory, uint256 elastic) {
        elastic = toElastic(total, base, roundUp);
        total.elastic -= uint128(elastic);
        total.base -= uint128(base);
        return (total, elastic);
    }

    /// @notice Add `elastic` and `base` to `total`.
    function add(
        Rebase memory total,
        uint256 elastic,
        uint256 base
    ) internal pure returns (Rebase memory) {
        total.elastic += uint128(elastic);
        total.base += uint128(base);
        return total;
    }

    /// @notice Subtract `elastic` and `base` to `total`.
    function sub(
        Rebase memory total,
        uint256 elastic,
        uint256 base
    ) internal pure returns (Rebase memory) {
        total.elastic -= uint128(elastic);
        total.base -= uint128(base);
        return total;
    }

    /// @notice Add `elastic` to `total` and update storage.
    /// @return newElastic Returns updated `elastic`.
    function addElastic(
        Rebase storage total,
        uint256 elastic
    ) internal returns (uint256 newElastic) {
        newElastic = total.elastic += uint128(elastic);
    }

    /// @notice Subtract `elastic` from `total` and update storage.
    /// @return newElastic Returns updated `elastic`.
    function subElastic(
        Rebase storage total,
        uint256 elastic
    ) internal returns (uint256 newElastic) {
        newElastic = total.elastic -= uint128(elastic);
    }
}

contract BIGBANG_MOCK {
    using RebaseLibrary for Rebase;
    uint256 collateralizationRate; // 75%
    uint256 public liquidationMultiplier = 12000; //12%
    uint256 internal constant FEE_PRECISION = 1e5;
    uint256 public borrowOpeningFee = 50; //0.05%
    Rebase public totalBorrow;

    constructor(uint256 _collateralizationRate) {
        collateralizationRate = _collateralizationRate;
    }

    function computeClosingFactor(
        uint256 borrowPart,
        uint256 collateralPartInAsset,
        uint256 borrowPartDecimals,
        uint256 collateralPartDecimals,
        uint256 ratesPrecision
    ) public view returns (uint256) {
        uint256 borrowPartScaled = borrowPart;
        if (borrowPartDecimals > 18) {
            borrowPartScaled = borrowPart / (10 ** (borrowPartDecimals - 18));
        }
        if (borrowPartDecimals < 18) {
            borrowPartScaled = borrowPart * (10 ** (18 - borrowPartDecimals));
        }

        uint256 collateralPartInAssetScaled = collateralPartInAsset;
        if (collateralPartDecimals > 18) {
            collateralPartInAssetScaled =
                collateralPartInAsset /
                (10 ** (collateralPartDecimals - 18));
        }
        if (collateralPartDecimals < 18) {
            collateralPartInAssetScaled =
                collateralPartInAsset *
                (10 ** (18 - collateralPartDecimals));
        }

        uint256 liquidationStartsAt = (collateralPartInAssetScaled *
            collateralizationRate) / (10 ** ratesPrecision);
        if (borrowPartScaled < liquidationStartsAt) return 0;

        uint256 numerator = borrowPartScaled -
            ((collateralizationRate * collateralPartInAssetScaled) /
                (10 ** ratesPrecision));
        uint256 denominator = ((10 ** ratesPrecision) -
            (collateralizationRate *
                ((10 ** ratesPrecision) + liquidationMultiplier)) /
            (10 ** ratesPrecision)) * (10 ** (18 - ratesPrecision));

        // Here is denominator broken down for simpler picturing.
        // i left it out so you can play around with numbers and see outpput
        // uint256 denominator = (1e5 - (75000 * (1e5 + 12000)) / 1e5) * (1e13);
        // uint256 collateralizationRate_ = 89285; // 89286 cause Arithmetic over/underflow or Division or modulo by 0
        // uint256 y = (collateralizationRate_ * (1e5 + 12000)) / 1e5;
        // uint256 denominator = (1e5 - y) * (1e13);

        // console.log("denominator = 1e5 - %d * 1e13", y);

        // console.log("denominator: %d", denominator);
        uint256 x = (numerator * 1e18) / denominator;
        return x;
    }

    // copied from YieldBoxRebase
    function _toAmount(
        uint256 share,
        uint256 totalShares_,
        uint256 totalAmount,
        bool roundUp
    ) external pure returns (uint256 amount) {
        // To prevent reseting the ratio due to withdrawal of all shares, we start with
        // 1 amount/1e8 shares already burned. This also starts with a 1 : 1e8 ratio which
        // functions like 8 decimal fixed point math. This prevents ratio attacks or inaccuracy
        // due to 'gifting' or rebasing tokens. (Up to a certain degree)
        totalAmount++;
        totalShares_ += 1e8;

        // Calculte the amount using te current amount to share ratio
        amount = (share * totalAmount) / totalShares_;

        // Default is to round down (Solidity), round up if required
        if (roundUp && (amount * totalShares_) / totalAmount < share) {
            amount++;
        }
    }
}

contract BIGBANG_ISSUES is DSTest, Test {
    BIGBANG_MOCK bigbangMock_75000_colRate;
    BIGBANG_MOCK bigbangMock_89285_colRate;
    BIGBANG_MOCK bigbangMock_89286_colRate;
    BIGBANG_MOCK bigbangMock_90000_colRate;

    function setUp() public {
        bigbangMock_75000_colRate = new BIGBANG_MOCK(75000);
        bigbangMock_89285_colRate = new BIGBANG_MOCK(89285);
        bigbangMock_89286_colRate = new BIGBANG_MOCK(89286);
        bigbangMock_90000_colRate = new BIGBANG_MOCK(90000);
    }

    // with lower than 89286: Works as intended
    function test_colRate_lower_than_89286_works() public {
        //
        uint256 EXCHANGE_RATE_PRECISION = 1e18;
        uint256 _exchangeRate = 1e18; // i.e. 1$ => price of USDO

        uint256 borrowed = 100e18; //;
        uint256 userCollateralShare = 10e18;
        uint256 totalShares_ = 100e18;
        uint256 totalAmount = 1000e18;

        uint256 collateralPartInAsset = (bigbangMock_75000_colRate._toAmount(
            userCollateralShare,
            totalShares_,
            totalAmount,
            false
        ) * EXCHANGE_RATE_PRECISION) / _exchangeRate;

        uint256 borrowAssetDecimals = 18;
        uint256 collateralDecimals = 18;
        uint256 FEE_PRECISION_DECIMALS = 5;
        uint256 availableBorrowPart = bigbangMock_75000_colRate
            .computeClosingFactor(
                borrowed,
                collateralPartInAsset,
                borrowAssetDecimals,
                collateralDecimals,
                FEE_PRECISION_DECIMALS
            );

        // console.log("availableBorrowPart: %d", availableBorrowPart);
        console.log("collateralizationRate 75000: works as intended");

        availableBorrowPart = bigbangMock_89285_colRate.computeClosingFactor(
            borrowed,
            collateralPartInAsset,
            borrowAssetDecimals,
            collateralDecimals,
            FEE_PRECISION_DECIMALS
        );

        // console.log("availableBorrowPart: %d", availableBorrowPart);
        console.log("collateralizationRate 89285: works as intended");
    }

    // Revert: Division or modulo by 0
    // Arithmetic over/underflow
    function test_colRate_greater_than_89286_will_revert() public {
        //
        uint256 EXCHANGE_RATE_PRECISION = 1e18;
        uint256 _exchangeRate = 1e18; // i.e. 1$ => price of USDO

        uint256 borrowed = 100e18; //;
        uint256 userCollateralShare = 10e18;
        uint256 totalShares_ = 100e18;
        uint256 totalAmount = 1000e18;

        uint256 collateralPartInAsset = (bigbangMock_89286_colRate._toAmount(
            userCollateralShare,
            totalShares_,
            totalAmount,
            false
        ) * EXCHANGE_RATE_PRECISION) / _exchangeRate;

        uint256 borrowAssetDecimals = 18;
        uint256 collateralDecimals = 18;
        uint256 FEE_PRECISION_DECIMALS = 5;

        // If you comment this out, you will get the error message
        vm.expectRevert(); // Reason: Division or modulo by 0
        uint256 availableBorrowPart = bigbangMock_89286_colRate
            .computeClosingFactor(
                borrowed,
                collateralPartInAsset,
                borrowAssetDecimals,
                collateralDecimals,
                FEE_PRECISION_DECIMALS
            );
        console.log(
            "collateralizationRate 89286: revert with Division or modulo by 0"
        );

        // If you comment this out, you will get the error message
        vm.expectRevert(); // Reason: Arithmetic over/underflow
        availableBorrowPart = bigbangMock_90000_colRate.computeClosingFactor(
            borrowed,
            collateralPartInAsset,
            borrowAssetDecimals,
            collateralDecimals,
            FEE_PRECISION_DECIMALS
        );
        console.log(
            "collateralizationRate 9000: revert with Arithmetic over/underflow"
        );
    }
}

Tools Used

Manual analysis

Recommended Mitigation Steps

Make sure the value that is deducted from (10 ** ratesPrecision) is lower than (10 ** ratesPrecision)

Assessed type

Other

Lines of code

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

Vulnerability details

Impact

No protection for the protocol swap when _liquidateUser() is called, and that could cause loss for the protocol (For example, because of a sandwich attack occurrence ..etc).

Proof of Concept

In liquidate() method, _closedLiquidation() is called which will call _liquidateUser() to handle the liquidations of insolvent users. As you see below there is a swap done after building swapData

        uint256 minAssetMount = 0;
        if (_dexData.length > 0) {
            minAssetMount = abi.decode(_dexData, (uint256));
        }


        uint256 balanceBefore = yieldBox.balanceOf(address(this), assetId);


        ISwapper.SwapData memory swapData = swapper.buildSwapData(
            collateralId,
            assetId,
            0,
            collateralShare,
            true,
            true
        );
        swapper.swap(swapData, minAssetMount, address(this), "");

Code link

This is to swap the collateral to asset in order to pay the debt. However, swap here is relying on swapData and minAssetMount, and if _dexData is empty then minAssetMount will be zero

uint256 minAssetMount = 0;
        if (_dexData.length > 0) {
            minAssetMount = abi.decode(_dexData, (uint256));
        }

Code link

if minAssetMount happened to be zero then the result of swap might has loss unexpectedly since there is no protection for how minimum amount the protocol should receive from swapping.

swapper.swap(swapData, minAssetMount, address(this), "");

Note: Attacker could be even the liquidator him/her self

Tools Used

Manual Review

Recommended Mitigation Steps

Check minAssetMount if it is zero after _dexData condition, then set a dynamic default value for it (based on the exchange rate) before the swap. For example, 70% at least of the value as a minimum. Otherwise, it will be a loss for the protocol. Please consider making the percentage configurable in case you need to update it in future.

Assessed type

Other