code-423n4 / 2023-07-tapioca-findings

13 stars 9 forks source link

[MC01] Market liquidations can revert due to arithmetic underflow #1012

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

The Market.sol contract's computeClosingFactor function calculates how much of the collateral of an user can be liquidated to restore the user's account to a healthy state. The formula for this can be derived as follows:

Assume the user has a position with collateral $C$ and debt $D$, all denominated in USD. When being liquidated, the liquidator part of the debt $delta$ and takes part of the collateral $L*delta$, where $l$ is the liquidation incentive.

The new position becomes: Collateral = $C-L*delta$, Debt = $D-delta$

Health Factor = Collateral / Debt = ($C-Ldelta$)$CR$ / ($D-delta$) = 1.0 (for healthy position), where $CR$ is the collateral ratio.

From this we can derive $delta$ as

$$delta = \frac{D-CRC}{1-CRL}$$

To bring this in line with the implementation in the contract, we can write $L$ as $1+l$, where $l$ is the liquidation incentive expressed as a fraction.

$$delta = \frac{D-CRC}{1-(1+l)CR}$$

This delta is calculated and stored in two variables numerator and denominator along with some decimal math. However it can be trivially seen that if $l>(1/CR - 1)$, then the denominator can be a negative number.

Similarly, if we look at the code snippet in the contract, we can see the same issue arise.

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

If the liquidationMultiplier and collateralizationRate satisfy the inequality described above, this term computed can be negative, and thus revert. for a simple case of liquidation incentive of 10%, the any collateralization rate above 91% will cause this to revert. This can be shown by setting liquidationMultiplier to 1e17 (10%), collateralizationRate to 91e16 (91%) and ratesPrecision to 18.

This function is called during the liquidation process, and if it reverts due to incorrect ranges of the set values, it can lead to un-liquidatable positions. Since this issue depends on the admin choosing certain values, it is classified as a medium severity issue.

Proof of Concept

The following snippet can be run on remix to show the underflow:

pragma solidity ^0.8.0;
contract Test{
    function test() public pure returns(uint256 denominator){

        uint256 ratesPrecision = 18;
        uint256 collateralizationRate = 91e16;
        uint256 liquidationMultiplier = 1e17;

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

}

This function reverts if collateralizationRate is set to 91e16 (91%) or higher. If set below 90e16, it works fine. Thus for certain combinations of collaterilaztionRate and liquidationMultipliers, positions can become unliquidatable.

Tools Used

Remix

Recommended Mitigation Steps

When setting the values of collateralizationRate and liquidationMultiplier, ensure that the inequality described above is not satisfied.

Assessed type

Math

c4-pre-sort commented 1 year ago

minhquanym marked the issue as primary issue

c4-sponsor commented 1 year ago

cryptotechmaker (sponsor) confirmed

c4-judge commented 1 year ago

dmvt marked the issue as selected for report