DittoETH

Ditto
DeFiFoundryOracle
55,000 USDC
View results
Submission Details
Severity: high
Invalid

Deferring flagged shorts from being liquidated by combining shorts after front-running the oracle price update

Summary

The ShortRecordFacet::combineShorts() is vulnerable to front-running attacks, as the function calculates the merged Short's collateral ratio using a cached price, which can be front-run by attackers.

As a result, attackers can combine their flagged Shorts with non-flagged Shorts to reset the liquidation flags even if the actual merged Short's collateral ratio might be below the primaryLiquidationCR threshold, deferring their flagged Short positions from being liquidated (Primary liquidation method).

Vulnerability Details

A shorter can execute the combineShorts() to combine their active Short positions into one Short. After all ShortRecord positions are merged into one Short (firstShort), if at least one Short was flagged for liquidation, the function will execute the LibShortRecord::getCollateralRatioSpotPrice() to calculate the merged Short's collateral ratio (cRatio).

If the resulting cRatio is above the primaryLiquidationCR threshold, the combineShorts() will invoke the LibShortRecord::resetFlag() to reset the liquidation flag of the merged Short position. Otherwise, the function will revert the transaction due to insufficient collateral (to be above the primaryLiquidationCR).

However, the combineShorts() is vulnerable to front-running attacks since the getCollateralRatioSpotPrice() will calculate the cRatio using a cached price retrieved from the getSavedOrSpotOraclePrice(), which can be front-run by attackers.

To elaborate on the vulnerability, please consider the following section.

Explaining how the combineShorts() can be attacked by front-running

The combineShorts() executes the getSavedOrSpotOraclePrice() to query for the latest price from Chainlink if the last updated timestamp is more than or equal to 15 minutes. Otherwise, the function will return the cached oracle price. With the 15-minute update window, an attacker has room to front-run the protocol's oracle price update. In other words, soon after Chainlink has updated its price, the getSavedOrSpotOraclePrice() cannot guarantee that it will immediately fetch the latest price from Chainlink.

Let's say Chainlink has updated the price to be higher than the protocol's oracle price (cached). The attacker (i.e., shorter) can front-run the protocol's oracle price update and execute the combineShorts() to combine their flagged Shorts with non-flagged Shorts.

Since the protocol's oracle still retains the lower price (lower debt than the actual), the collateral and debt (ercDebt) of the flagged Shorts after being combined with non-flagged Shorts may bring the merged Short's cRatio to be above the primaryLiquidationCR threshold. This can occur because the cached price is lower than the actual price; if we consider the updated price fed by Chainlink, the merged Short's cRatio may be below the primaryLiquidationCR threshold (higher debt in reality).

This way, the attacker can defer their flagged Short positions from being liquidated by the MarginCallPrimaryFacet::liquidate() (Primary liquidation method). Note that to liquidate the attacker's merged Short again, a flagger must flag that merged Short again. It will take at least 10 hours (the default of the firstLiquidationTimeparameter) for the flagged Short to be liquidatable.

Consequently, this vulnerability can indirectly increase bad debt to the Ditto protocol. The protocol can become insolvent, and the protocol's minted stable assets (e.g., cUSD) can eventually be de-pegged.

// FILE: https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/facets/ShortRecordFacet.sol
function combineShorts(address asset, uint8[] memory ids)
external
isNotFrozen(asset)
nonReentrant
onlyValidShortRecord(asset, msg.sender, ids[0])
{
...
// Merge all short records into the short at position id[0]
firstShort.merge(ercDebt, ercDebtSocialized, collateral, yield, c.shortUpdatedAt);
// If at least one short was flagged, ensure resulting c-ratio > primaryLiquidationCR
if (c.shortFlagExists) {
if (
firstShort.getCollateralRatioSpotPrice(
@> LibOracle.getSavedOrSpotOraclePrice(_asset)
) < LibAsset.primaryLiquidationCR(_asset)
) revert Errors.InsufficientCollateral();
// Resulting combined short has sufficient c-ratio to remove flag
@> firstShort.resetFlag();
}
emit Events.CombineShorts(asset, msg.sender, ids);
}
// FILE: https://github.com/Cyfrin/2023-09-ditto/blob/main/contracts/libraries/LibOracle.sol
function getSavedOrSpotOraclePrice(address asset) internal view returns (uint256) {
@> if (LibOrders.getOffsetTime() - getTime(asset) < 15 minutes) {
@> return getPrice(asset);
} else {
return getOraclePrice(asset);
}
}
  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/ShortRecordFacet.sol#L182

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/facets/ShortRecordFacet.sol#L186

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/libraries/LibOracle.sol#L154

  • https://github.com/Cyfrin/2023-09-ditto/blob/a93b4276420a092913f43169a353a6198d3c21b9/contracts/libraries/LibOracle.sol#L155

Impact

This vulnerability enables an attacker (i.e., shorter) to combine their flagged Shorts with non-flagged Shorts to reset the liquidation flags even if the actual merged Short's collateral ratio might be below the primaryLiquidationCR threshold, deferring their flagged Short positions from being liquidated (Primary liquidation method).

Note that to liquidate the attacker's merged Short again, a flagger must flag that merged Short again. It will take at least 10 hours (the default of the firstLiquidationTimeparameter) for the flagged Short to be liquidatable.

The vulnerability can indirectly increase bad debt to the Ditto protocol. The protocol can become insolvent, and the protocol's minted stable assets (e.g., cUSD) can eventually be de-pegged.

Tools Used

Manual Review

Recommendations

Since the cached oracle price is prone to front-running attacks, always execute the LibOracle::getOraclePrice() to get the accurate price from Chainlink.

Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other
serialcoder Submitter
almost 2 years ago
0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.