c4-bot-2 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibSRUtil.sol#L115

Vulnerability details

Impact

Adding a short while the market is in recovery mode.

Proof of Concept

Assuming the current oracle price at the moment for the deth in dusdis too low (assetCR is very low) then the market now is in Recovery Mode.

checkRecoveryModeViolation is checking if a shorter is able to create a short order or not according to shortRecord CR. if shortRecord CR is too low then shorter shouldn't be able to create a short.

    uint256 assetCR = Asset.dethCollateral.div(oraclePrice.mul(Asset.ercDebt));
    if (assetCR < recoveryCR) {
        // Market is in recovery mode and shortRecord CR too low
        return true;
    }

A shorter can avoid checkRecoveryModeViolation by relying on an oracle savedPrice. let's say the current oracle price represent a recovery mode for the market but the old saved oracle price doesn't. since the shorter relied on previous saved oracle price (the price was healthy), he will be able to create short order and violate the market recovery mode.

How oracle price could be savedPrice when checkRecoveryModeViolation is called? getSavedOrSpotOraclePrice is being called before checkRecoveryModeViolation method and p.oraclePrice passed to it. In getSavedOrSpotOraclePrice a check is happening, that if we should update the oracle price according to creationTime of the asset (which gets updated when reading from Oracle) if (LibOrders.getOffsetTime() - getTime(asset) < 15 minutes), if It doesn't cross 15 mins then it will return getPrice which is the last saved oracle price not the current oracle price that comes from getOraclePrice.

As result a savedPrice can cause an issue for the market and allow a shorter to create a short while the market according to the current oracle price is in recovery mode.

Tools Used

Manual Review

Recommended Mitigation Steps

Use the current oracle price method getOraclePrice for checkRecoveryModeViolation instead of getSavedOrSpotOraclePrice so it skips the 15 mins delay.

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as duplicate of #114

raymondfam commented 5 months ago

Same root cause as in #114 leading to a differing outcome.

c4-judge commented 4 months ago

hansfriese marked the issue as satisfactory

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

Possible to avoid checkRecoveryModeViolation #241