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

0 stars 0 forks source link

Shorter can force Recovery Mode to push other users into liquidation #255

Closed c4-bot-1 closed 4 months ago

c4-bot-1 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L47-L90 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L102-L122 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L22-L47

Vulnerability details

Summary

A shorter can repay the ercDebt on their Short Record to trigger Recovery Mode and then liquidate users

Vulnerability Details

The idea behind Recovery Mode is a higher CR limit, which is imposed on Short Records when certain functions are called on them. Whether the higher CR limit gets imposed is determined by checkRecoveryModeViolation() and the calculation of an Asset's CR as a whole:

uint256 assetCR = Asset.dethCollateral.div(oraclePrice.mul(Asset.ercDebt));

This effectively gives external control over the setting of recoveryCR. When a shorter reduces their Short Record's ercDebt they will cause Asset.dethCollateral to decrease also. This could be achieved by the repayment of Short Records with very healthy CRs via the short repayment functions in ExitShortFacet.sol which call disburseCollateral():

    function disburseCollateral(address asset, address shorter, uint88 collateral, uint256 dethYieldRate, uint32 updatedAt)
        internal
    {
        // SOME CODE

        Asset.dethCollateral -= collateral;

        // SOME CODE
    }

Although recoveryCR will equal liquidationCR initially for dUSD, this will not be the case for all assets, and maybe not dUSD in long term.

Impact

Being able to trigger a change of CR to a higher level can be very damaging to users of the protocol, allowing an attacker to build up large short positions before crashing assetCR and liquidatin users before they have time to adjust to the new CR requirement.

Tools Used

Manual Review Foundry Testing

Recommendations

Consider a reduced incentive for liquidators at this higher level

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

Inadequate proof to support your described issue.

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Insufficient proof