hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

Audit competition repository for Euro-Dollar (0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd)
https://hats.finance
MIT License
3 stars 2 forks source link

Price Oracle Manipulation Through Multiple Small Updates #28

Open hats-bug-reporter[bot] opened 3 weeks ago

hats-bug-reporter[bot] commented 3 weeks ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x80e80c19c9f14780d49700596673f32a3f25f27b6f197db4cf053ecf529515e9 Severity: high

Description: Description\ The YieldOracle contract implements a maxPriceIncrease check to prevent large price jumps, but this protection only considers the immediate price change:

function updatePrice(uint256 price) external onlyOracle {
    require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");
    if (nextPrice != NO_PRICE) {
        previousPrice = currentPrice;
        currentPrice = nextPrice;
        emit PriceCommitted(currentPrice);
    }
    require(price - currentPrice <= maxPriceIncrease, "Price out of bounds");
    nextPrice = price;
    lastUpdate = block.timestamp;
    emit PriceUpdated(price);
}

The flaw is that while each individual update is limited to maxPriceIncrease (default 0.1e18 or 10%), there's no restriction on cumulative price changes over time.

A malicious oracle could perform multiple small updates in succession to achieve a larger price movement than intended.

Attack Scenario\ Initial price is 1.0 (scaled to 1e18)

Oracle wants to manipulate price to 2.0, but maxPriceIncrease is 0.1e18 (10%)

Oracle executes multiple updates:

Update 1: 1.0 → 1.1 (+10%)

Update 2: 1.1 → 1.21 (+10%)

Update 3: 1.21 → 1.331 (+10%)

And so on until reaching target price

Each update passes the maxPriceIncrease check, but cumulative change is much larger

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

    
    // SPDX-License-Identifier: MIT
    pragma solidity ^0.8.21;

contract YieldOracle { // Add state variables for tracking price movement uint256 public constant WINDOW_SIZE = 1 days; uint256 public constant MAX_CUMULATIVE_CHANGE = 0.3e18; // 30% max change per day

struct PriceUpdate {
    uint256 timestamp;
    uint256 price;
}

// Circular buffer for price history
PriceUpdate[24] private priceHistory;
uint8 private currentIndex;

/**
 * @dev Records a price update in history
 * @param price New price being set
 */
function _recordPriceUpdate(uint256 price) private {
    priceHistory[currentIndex] = PriceUpdate({
        timestamp: block.timestamp,
        price: price
    });
    currentIndex = (currentIndex + 1) % 24;
}

/**
 * @dev Validates cumulative price change
 * @param newPrice Proposed new price
 * @return bool Whether the price change is acceptable
 */
function _validateCumulativeChange(uint256 newPrice) private view returns (bool) {
    // Find oldest price within window
    uint256 oldestRelevantPrice = currentPrice;
    uint256 windowStart = block.timestamp - WINDOW_SIZE;

    for(uint8 i = 0; i < 24; i++) {
        if(priceHistory[i].timestamp > windowStart) {
            oldestRelevantPrice = priceHistory[i].price;
            break;
        }
    }

    // Calculate absolute percentage change
    uint256 priceChange;
    if(newPrice > oldestRelevantPrice) {
        priceChange = ((newPrice - oldestRelevantPrice) * 1e18) / oldestRelevantPrice;
    } else {
        priceChange = ((oldestRelevantPrice - newPrice) * 1e18) / oldestRelevantPrice;
    }

    return priceChange <= MAX_CUMULATIVE_CHANGE;
}

/**
 * @notice Updates the price with cumulative change validation
 * @param price The new price to be set
 */
function updatePrice(uint256 price) external onlyOracle {
    require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");

    // Check immediate price change
    require(price - currentPrice <= maxPriceIncrease, "Immediate price change too high");

    // Check cumulative price change
    require(_validateCumulativeChange(price), "Cumulative price change too high");

    if (nextPrice != NO_PRICE) {
        previousPrice = currentPrice;
        currentPrice = nextPrice;
        emit PriceCommitted(currentPrice);
    }

    nextPrice = price;
    lastUpdate = block.timestamp;

    _recordPriceUpdate(price);

    emit PriceUpdated(price);
}

/**
 * @notice Get price change over specified window
 * @param window Time window in seconds to check
 * @return change Absolute percentage change in price
 */
function getPriceChange(uint256 window) external view returns (uint256 change) {
    require(window <= WINDOW_SIZE, "Window too large");
    // Implementation details...
}

}



The revised code adds:

Price history tracking using a circular buffer

Cumulative price change validation over a time window

Maximum allowed price change over the window

Helper functions to track and query price changes

Maintains existing immediate price change checks
AndreiMVP commented 3 weeks ago

This is known and intended behaviour. Idea is we update maxPriceIncrease periodically. As mentioned in the README, that variable is a:

Guard rail to ensure that a faulty oracle bot does not increase price arbitrarily maxPriceIncrease.