hats-finance / Euro-Dollar-0xa4ccd3b6daa763f729ad59eae75f9cbff7baf2cd

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

Oracle updatePrice can be front run for value extraction #16

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf9aa717628e796693ae5526e037c0762499c8675bebb05e3f34dea348aaeb9fd Severity: medium

Description: Description\ The updatePrice() and commitPrice() functions can be frontrun and convert assets to shares, then when the new price is updated, the shares can be converted to assets for instant profit.

Attack Scenario\

  1. Oracle updates price to a higher value
  2. Attacker frontruns the oracle
  3. The new price is commited
  4. The attacker profits by converting the shares to assets

Attachments

  1. Proof of Concept (PoC) File
function updatePrice(uint256 price) external onlyOracle {
    // Enforce at least updateDelay between updates
    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);
}

/**
 * @notice Commits the price.
 */
function commitPrice() public {
    require(nextPrice - currentPrice >= 0, "Price out of bounds");

    // Enforce at least commitDelay after the last update
    require(lastUpdate + commitDelay < block.timestamp, "Insufficient commit delay");

    previousPrice = currentPrice;
    currentPrice = nextPrice;
    nextPrice = NO_PRICE;

    emit PriceCommitted(currentPrice);
}
  1. Revised Code File (Optional) Make it so the updated price is linear instead of instant.
AndreiMVP commented 1 week ago

The price is only increasing, but in case of someone trying to steal the new price yield, selling would be done at previous price until next update (after update delay.), hence the current/previous price logic.

JordanRaphael commented 1 week ago

@AndreiMVP The attacker who wants to steal the yield has to wait commitDelay which starts at 1 hour. In order to avoid such attacks you'd have a period in which the price is updated linearly, e.g. after 1 hour of calling updatePrice it starts increasing linearly for the next 24 hours.