code-423n4 / 2024-03-dittoeth-findings

0 stars 0 forks source link

Liquidation could be blocked if there is not enough ethEscrowed in TAPP #247

Closed c4-bot-8 closed 4 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L225

Vulnerability details

Impact

Liquidation could be blocked in some cases caused by _liquidationFeeHandler

Proof of Concept

liquidate method calls _liquidationFeeHandler method to handle the fees for the caller. The caller receives the fee from the TAPP.ethEscrowed

    TAPP.ethEscrowed -= callerFee;
    VaultUser.ethEscrowed += callerFee;

However, in case there’s not enough ethEscrowed of TAPP, The caller is given (portion of?) tappFee instead of gasFee.

        // Give caller (portion of?) tappFee instead of gasFee
        VaultUser.ethEscrowed += callerFee - m.gasFee + tappFee;
        m.totalFee -= m.gasFee;
        TAPP.ethEscrowed -= m.totalFee;

The issue is, if TAPP.ethEscrowed is lower than m.ethFilled.mulU88(m.callerFeePct) then this line will revert

VaultUser.ethEscrowed += callerFee;

as you see totalFee math:

m.totalFee += tappFee + callerFee;

Here is a very simplified example (numbers for demo only): tappFee = 10 m.ethFilled.mulU88(m.callerFeePct) = 30 m.gasFee = 40 TAPP.ethEscrowed = 20

This means callerFee = 30 + 40 = 70 m.totalFee = 10 + 70 = 80

Now let's execute this code

        m.totalFee -= m.gasFee;
        TAPP.ethEscrowed -= m.totalFee;

m.totalFee = m.totalFee - 40 = 40 TAPP.ethEscrowed = TAPP.ethEscrowed - 40 = 20 - 40

As you can see, a revert is occuring here because TAPP.ethEscrowed is lower than m.totalFee after previous calculations.

Tools Used

Manual review

Recommended Mitigation Steps

Add an option to allow liquidation without the caller receiving fees, this is good for the protocol. In case a liquidation has to be done, it should be possible in any case if the caller chooses not to take fee.

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

raymondfam commented 5 months ago

Similar to the known issue: When there are few ShortRecords or TAPP is low, it's easy to fall into a black swan scenario

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Out of scope