code-423n4 / 2024-03-revert-lend-findings

13 stars 10 forks source link

Incorrect liquidation fee calculation during underwater liquidation, disincentivizing liquidators to participate #53

Open c4-bot-1 opened 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1112-L1119

Vulnerability details

Bug Description

As stated in the Revert Lend Whitepaper, the liquidation fee for underwater positions is supposed to be 10% of the debt.

However the code within V3Vault::_calculateLiquidation (shown below) calculates the liquidation fee as 10% of the fullValue rather than 10% of the debt.

        } else {
            // all position value
            liquidationValue = fullValue;

            uint256 penaltyValue = fullValue * (Q32 - MAX_LIQUIDATION_PENALTY_X32) / Q32;
            liquidatorCost = penaltyValue;
            reserveCost = debt - penaltyValue;
        }

Note: fullValue * (Q32 - MAX_LIQUIDATION_PENALTY_X32) / Q32; is equivalent to fullValue * 90%.

A permalink to the code snippet is here

Impact

As the fullValue decreases below debt (since the position is underwater), liquidators are less-and-less incentivised to liquidate the position. This is because as fullValue decreases, the liquidation fee (10% of fullValue) also decreases.

This goes against the protocol's intention (stated in the whitepaper) that the liquidation fee will be fixed at 10% of the debt for underwater positions, breaking core protocol functionality.

Proof of Concept

Code snippet from V3Vault._calculateLiquidation: https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L1112-L1119

Recommended Mitigation Steps

Ensure that the liquidation fee is equal to 10% of the debt. Make the following changes in V3Vault::_calculateLiquidation():

else {
-// all position value
-liquidationValue = fullValue;

-uint256 penaltyValue = fullValue * (Q32 - MAX_LIQUIDATION_PENALTY_X32) / Q32;
-liquidatorCost = penaltyValue;
-reserveCost = debt - penaltyValue;

+uint256 penalty = debt * (MAX_LIQUIDATION_PENALTY_X32) / Q32; //[10% of debt]
+liquidatorCost = fullValue - penalty;
+liquidationValue = fullValue;
+reserveCost = debt - liquidatorCost; // Remaining to pay. 
}   

Assessed type

Error

c4-pre-sort commented 7 months ago

0xEVom marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

0xEVom marked the issue as primary issue

0xEVom commented 7 months ago

Medium severity at best, liquidators are still incentivized to participate.

c4-sponsor commented 7 months ago

kalinbas (sponsor) confirmed

c4-sponsor commented 7 months ago

kalinbas marked the issue as disagree with severity

kalinbas commented 7 months ago

Low severity

jhsagd76 commented 7 months ago

According to the C4 rules, an M is appropriate as this disrupts certain designs in the economic model.

c4-judge commented 7 months ago

jhsagd76 changed the severity to 2 (Med Risk)

c4-judge commented 7 months ago

jhsagd76 marked the issue as satisfactory

c4-judge commented 7 months ago

jhsagd76 marked the issue as selected for report

kalinbas commented 7 months ago

https://github.com/revert-finance/lend/pull/7