Financial losses for users possible due to inaccurate liquidations or redemptions.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L135 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/facets/PrimaryLiquidationFacet.sol#L47-L90 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L122-L143

Vulnerability details

Reliance on a single price oracle for critical functions, such as liquidations and redemptions, makes it vulnerable to oracle manipulation or unreliability, potentially leading to financial losses for users.

• The liquidate function relies on a single oracle price obtained through LibOracle.getPrice(asset) to determine the collateral ratio of the short position.
• If the oracle price is manipulated or becomes stale, it can lead to incorrect collateral ratio calculations and improper liquidations.
• The vulnerable line 135

   m.oraclePrice = LibOracle.getPrice(asset);

Issue Description

• liquidate function in the PrimaryLiquidationFacet.sol contract is responsible for liquidating undercollateralized short positions.
• It relies on the LibOracle.getPrice function to obtain the current price of the asset from the oracle.

• liquidate, _setLiquidationStruct

   function liquidate(address asset, address shorter, uint8 id, uint16[] memory shortHintArray)
       external
       isNotFrozen(asset)
       nonReentrant
       onlyValidShortRecord(asset, shorter, id)
       returns (uint88, uint88)
   {
       // ...
  @>       MTypes.PrimaryLiquidation memory m = _setLiquidationStruct(asset, shorter, id);
       // ...
   }
  
   function _setLiquidationStruct(address asset, address shorter, uint8 id)
       private
       returns (MTypes.PrimaryLiquidation memory)
   {
       // ...
  @>       m.oraclePrice = LibOracle.getPrice(asset);
  @>       m.cRatio = m.short.getCollateralRatio(m.oraclePrice);
       // ...
   }

  The liquidate function is expected to accurately determine the collateral ratio of a short position using the current market price provided by a reliable oracle.

• If the collateral ratio falls below the liquidation threshold, the function should proceed with the liquidation process to maintain the stability of the protocol.

• If the oracle price is manipulated or becomes stale, the liquidate function may make incorrect decisions based on the inaccurate price data.
• This can lead to scenarios where:
• Undercollateralized positions are not liquidated properly, allowing them to remain open and potentially putting the protocol at risk.
• Sufficiently collateralized positions are incorrectly liquidated, causing unwarranted losses for users.
• If the conversion from ETH or LSTs to dETH is not handled accurately, it could lead to discrepancies in user balances.
• Improper storage or management of deposited funds in Vault and Bridge contracts could result in fund loss.

Impact

• Users may suffer financial losses due to incorrect liquidations or the failure to liquidate undercollateralized positions.
• The stability and integrity of the protocol can be compromised, as the system relies on accurate price data to maintain collateralization levels and enforce liquidations.

Proof of Concept

Let's see a scenario of how reliance on a single oracle in the DittoETH protocol can lead to incorrect liquidations and potential financial losses for users.

1. Alice opens a short position with an initial collateral ratio of 200% (2x collateralization) on the DittoETH protocol.
2. The current market price of the asset is $100, and the oracle provides the same price to the protocol.
3. The liquidation threshold for short positions is set to 150% collateral ratio.
4. An attacker manipulates the oracle price feed, causing it to report an incorrect price of $150 to the protocol, while the actual market price remains at $100.

PoC Steps:

1. The attacker manipulates the Oracle price feed, causing it to report an incorrect price of $150 to the protocol.

2. The liquidate function is called for Alice's short position.

3. Inside the liquidate function, the _setLiquidationStruct function is invoked to retrieve the oracle price and calculate the collateral ratio.

4. The LibOracle.getPrice(asset) function returns the manipulated price of $150, which is used to calculate the collateral ratio.

5. Due to the manipulated oracle price, the calculated collateral ratio for Alice's position falls below the liquidation threshold of 150%, even though the actual market price hasn't changed.

6. The liquidate function proceeds with the liquidation process, incorrectly liquidating Alice's sufficiently collateralized position.

7. Alice suffers unwarranted financial losses as a result of the incorrect liquidation.

Code Snippet:

function _setLiquidationStruct(address asset, address shorter, uint8 id)
    private
    returns (MTypes.PrimaryLiquidation memory)
{
    // ...
@>  m.oraclePrice = LibOracle.getPrice(asset); // Retrieves the manipulated price of $150
@>  m.cRatio = m.short.getCollateralRatio(m.oraclePrice); // Calculates the collateral ratio using the manipulated price
    // ...
}

Impact:

• Alice's sufficiently collateralized position is incorrectly liquidated due to the manipulated oracle price.
• Alice suffers financial losses as her position is liquidated at an unfavorable price, even though her actual collateral ratio is above the liquidation threshold.
• The protocol's stability is compromised as it fails to accurately assess the collateral ratios and make correct liquidation decisions.
• Users lose trust in the protocol as their positions become vulnerable to oracle manipulation.

Tools Used

Manual Audit

Recommended Mitigation Steps

• Implement a multi-oracle approach:

   function _getOraclePrice(address asset) private view returns (uint256) {
       uint256 primaryPrice = LibOracle.getPrice(asset);
       uint256 secondaryPrice = AltOracle.getPrice(asset);
       uint256 tertiaryPrice = BackupOracle.getPrice(asset);
  
       // Compare prices and use aggregation logic (e.g., median)
       // ...
  
       return aggregatedPrice;
   }

  • Introduce price deviation checks and circuit breakers:

    function _validateOraclePrice(uint256 oraclePrice, uint256 referencePrice) private view {
     uint256 maxDeviation = referencePrice.mul(MAX_DEVIATION_PERCENTAGE).div(100);
     require(
         oraclePrice >= referencePrice.sub(maxDeviation) &&
         oraclePrice <= referencePrice.add(maxDeviation),
         "Oracle price deviation exceeds threshold"
     );
    }

  • Implement a fallback oracle mechanism:

    function _getOraclePrice(address asset) private view returns (uint256) {
     uint256 primaryPrice = LibOracle.getPrice(asset);
     if (_isReliable(primaryPrice)) {
         return primaryPrice;
     }
    
     uint256 fallbackPrice = FallbackOracle.getPrice(asset);
     if (_isReliable(fallbackPrice)) {
         return fallbackPrice;
     }
    
     // Resort to other fallback mechanisms or revert
     // ...
    }

Assessed type

Oracle

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[raymondfam]

Check the LibOracle. It already has fallback to a Uniswap TWAP.

[c4-judge]

hansfriese marked the issue as unsatisfactory: Invalid