Summary

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

As a result, an attacker can withdraw collateral more than the actual (below the actual initialMargin threshold).

Vulnerability Details

A shorter can execute the decreaseCollateral() to decrease collateral (zETH) of their active Short position. After decreasing the collateral, the decreaseCollateral() will execute the LibShortRecord::getCollateralRatio() to calculate the Short's collateral ratio (cRatio). If the resulting cRatio is below the initialMargin threshold, the function will revert the transaction. In other words, the protocol does not permit a shorter to decrease their collateral below the initial margin.

However, the decreaseCollateral() is vulnerable to front-running attacks since the getCollateralRatio() will calculate the cRatio using a cached price retrieved from the LibOracle::getPrice().

Let's say Chainlink has updated the price to be higher than the protocol's oracle price (cached). An attacker (i.e., shorter) can front-run the protocol's oracle price update and execute the decreaseCollateral(). Since the protocol's oracle still retains the lower price (lower debt than the actual), the attacker can withdraw the collateral more than expected (below the actual initialMargin threshold).

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

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

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

Impact

This vulnerability enables an attacker (i.e., shorter) to withdraw collateral more than the actual (below the actual initialMargin threshold), breaking the protocol's invariant, which can render the protocol's functions and algorithms malfunctioning.

Consequently, 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.