Stale cached prices may misprice assets, delay liquidations, and increase risk.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L22 https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/libraries/LibOracle.sol#L19-L67

Vulnerability details

If the market experiences significant price volatility within the 15-minute window, the cached price may diverge from the actual market price. This discrepancy can lead to several issues:

• Incorrect calculation of collateral ratios:

  • If the cached price is higher than the actual market price, undercollateralized positions may remain open, exposing the protocol to increased risk.
  • If the cached price is lower than the actual market price, positions may be prematurely liquidated, causing unnecessary losses for users.

• Mispricing of assets during order matching:

  • If the cached price is stale, orders may be matched at prices that do not reflect the current market conditions.
  • This can result in unfair trades, where one party may receive an advantageous price at the expense of the other.
  • Malicious actors may exploit this by manipulating the market during the cache duration to benefit from the stale prices.

• Delayed liquidations of risky positions:

  • If the cached price is higher than the actual market price, positions that have fallen below the liquidation threshold may not be promptly identified and liquidated.
  • This delay in liquidation can expose the protocol to increased risk and potential losses.

It's from the following: LibOracle.sol#L22

uint256 protocolPrice = getPrice(asset);
// ... Oracle price retrieval logic ...

The getPrice function retrieves the cached price, which may be stale if the cache duration has not expired.

Impact

• Users may suffer financial losses due to incorrect liquidations or unfair pricing during order matching.
• The protocol may be exposed to increased risk if undercollateralized positions are not promptly identified and liquidated.

Description

getOraclePrice is responsible for retrieving the asset price from the oracle. It first attempts to fetch the price from the Chainlink oracle and falls back to a TWAP price from Uniswap if the Chainlink price is invalid or outdated.

LibOracle.sol#getOraclePrice

function getOraclePrice(address asset) internal view returns (uint256) {
    AppStorage storage s = appStorage();
    AggregatorV3Interface baseOracle = AggregatorV3Interface(s.baseOracle);
    uint256 protocolPrice = getPrice(asset);        <---------@

    AggregatorV3Interface oracle = AggregatorV3Interface(s.asset[asset].oracle);           <---------@
    if (address(oracle) == address(0)) revert Errors.InvalidAsset();         <---------@

    try baseOracle.latestRoundData() returns (uint80 baseRoundID, int256 basePrice, uint256, uint256 baseTimeStamp, uint80) {  <---------@
        // ... Chainlink price retrieval and validation logic ...
    } catch {
        if (oracle == baseOracle) {
            return twapCircuitBreaker();
        } else {
            // ... Fallback to TWAP price ...
        }
    }
}

retrieves the cached price and the setPriceAndTime function updates the cached price and timestamp. Is expected to retrieve the most up-to-date and accurate price for an asset from the oracle. The retrieved price should reflect the current market conditions and be used for various calculations and checks within the protocol.

Note:

The 15-minute cache duration, the getOraclePrice function may deviate from the expected behavior. If the market price changes significantly within the 15-minute window, the cached price may not accurately represent the current market conditions.

This can lead to incorrect calculations, mispricing of assets, and delayed liquidations, as described in the vulnerability details section.

Proof of Concept

Let's consider a hypothetical scenario where the DittoETH protocol is used for trading a stablecoin pegged to the US dollar (e.g., USDC) against ETH. The protocol relies on the Chainlink ETH/USD price oracle to determine the current market price of ETH.

Assume the following initial conditions:

• The cached ETH/USD price is $2,000, and the cache was last updated 14 minutes ago.
• The actual market price of ETH has suddenly dropped to $1,500 due to market volatility.
• Alice has a short position with a collateral ratio of 1.5, just above the liquidation threshold of 1.4.
• Bob is a malicious actor who wants to exploit the stale cached price.

Then takes these steps:

1. Bob observes that the cached ETH/USD price in the DittoETH protocol is stale and significantly higher than the actual market price. The protocol is still using the cached price of $2,000, while the market price has dropped to $1,500.

2. Bob quickly opens a large short position on the ETH/USD pair, taking advantage of the stale cached price. Since the protocol calculates the collateral ratio based on the cached price, Bob's short position appears to be sufficiently collateralized.

3. Bob then manipulates the market by placing a large sell order on an external exchange, driving the market price of ETH further down to $1,200.

4. Alice's short position, which was initially above the liquidation threshold, is now undercollateralized based on the actual market price. However, since the DittoETH protocol is still using the stale cached price of $2,000, Alice's position appears to be adequately collateralized, and no liquidation is triggered.

5. Bob waits for the cache duration to expire and the protocol to update the cached price to the current market price of $1,200. At this point, Alice's position becomes severely undercollateralized.

6. Bob quickly buys back the ETH at the lower price, closing his short position at a significant profit. Meanwhile, Alice's position remains open and is at risk of liquidation.

7. When the protocol finally updates the cached price and detects Alice's undercollateralized position, it triggers a liquidation. However, due to the delayed liquidation, the protocol may struggle to find sufficient liquidity to close Alice's position at the current market price, potentially leading to a cascade of liquidations and further price instability.

Consequences:

• Alice suffers a significant loss due to the delayed liquidation of her undercollateralized position. She may lose a substantial portion of her collateral.
• The protocol itself is exposed to increased risk, as it may not be able to liquidate undercollateralized positions effectively, leading to potential bad debt and losses for the protocol and its users.
• The overall stability and integrity of the DittoETH protocol are compromised, as the stale cached prices can be exploited by malicious actors to gain an unfair advantage and manipulate the market.

function getOraclePrice(address asset) internal view returns (uint256) {
    uint256 cachedPrice = getPrice(asset);
    uint256 cacheTimestamp = getPriceTimestamp(asset);

    if (block.timestamp - cacheTimestamp > 15 minutes) {
        // Cache duration expired, update the cached price
        uint256 currentPrice = fetchCurrentPrice(asset);
        setPrice(asset, currentPrice);
        setPriceTimestamp(asset, block.timestamp);
        return currentPrice;
    } else {
        // Use the cached price
        return cachedPrice;
    }
}

If the cache duration has not expired (i.e., less than 15 minutes have passed since the last price update), the function returns the cached price (cachedPrice). However, if the market price has changed significantly during this period, the returned price may be stale and not reflect the current market conditions, leading to the issues described in the vulnerability details section.

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing the mitigation measures discussed earlier, such as reducing the cache duration, implementing price deviation checks, using multiple Oracle price feeds, and regularly updating the Oracle integration to ensure the most accurate and up-to-date prices are used for various calculations and decisions within the protocol.

The price deviation check can be implemented:

function getOraclePrice(address asset) internal view returns (uint256) {
    // ... Existing code ...

+   uint256 cachedPrice = getPrice(asset);
+   uint256 currentPrice = fetchCurrentPrice(asset); // Retrieve the current price from the oracle

+   if (cachedPrice > 0) {
+       uint256 deviation = (currentPrice > cachedPrice) ? currentPrice - cachedPrice : cachedPrice - currentPrice;
+       if (deviation * 100 / cachedPrice > 10) { // Check if deviation exceeds 10%
+           // Trigger an alert or emergency action
            // ...
        }
    }

    // ... Existing code ...
}

As can be seen, the getOraclePrice function compares the cached price with the current price fetched from the oracle. If the deviation between the two prices exceeds a threshold (e.g., 10%), an alert or emergency action can be triggered to handle the significant price movement.

Assessed type

Other

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #114

[raymondfam]

See #114.

[c4-judge]

hansfriese marked the issue as satisfactory