005.md

Calm Sky Hippo

Medium

Incorrect time validation in currentValue function allows stale prices to be used or valid prices to be rejected

Summary

The currentValue function of the PythOracle contract has an incorrect time validation logic. The condition to check if the price data is "too old" fails due to a logic error in the require statement. This allows stale prices to be used or valid prices to be rejected.

Root Cause

The issue lies in the require condition within the currentValue function: https://github.com/sherlock-audit/2024-11-oku/blob/main/oku-custom-order-types/contracts/oracle/External/PythOracle.sol#L26-L33

require(
    price.publishTime < block.timestamp - noOlderThan,
    "Stale Price"
);

This logic incorrectly interprets what constitutes a "stale" price. It checks if publishTime is less than block.timestamp - noOlderThan, which is the opposite of what is intended. The correct logic should ensure that publishTime is greater than or equal to block.timestamp - noOlderThan, guaranteeing the price is fresh enough.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

1. Assume: block.timestamp is 10,000. noOlderThan is 100. price.publishTime is 9,950.

2. The price should be valid because 9,950 >= 10,000 - 100.

3. The require statement rejects the price because 9,950 < 9,900 is false.

Impact

Rejecting valid prices can lead to missed trading opportunities or incorrect oracle outputs, affecting any dependent systems. Also, incorrect logic causes valid prices to be rejected and shows the expected behavior when the logic is corrected.

PoC

const { expect } = require("chai");
const { ethers } = require("hardhat");

describe("PythOracle", function () {
  let PythOracle, pythOracle, MockPyth, mockPyth;
  let owner;

  beforeEach(async function () {
    // Get accounts
    [owner] = await ethers.getSigners();

    // Deploy MockPyth contract
    MockPyth = await ethers.getContractFactory("MockPyth");
    mockPyth = await MockPyth.deploy();
    await mockPyth.deployed();

    // Deploy PythOracle contract
    PythOracle = await ethers.getContractFactory("PythOracle");
    pythOracle = await PythOracle.deploy(
      mockPyth.address,           // pythOracle address
      ethers.utils.formatBytes32String("mockToken"), // tokenId
      100,                        // noOlderThan
      ethers.constants.AddressZero // underlying (mock value)
    );
    await pythOracle.deployed();
  });

  it("should reject valid prices due to incorrect time validation", async function () {
    // Set block.timestamp to 10,000
    await ethers.provider.send("evm_setNextBlockTimestamp", [10000]);
    await ethers.provider.send("evm_mine");

    // Set publish time to 9,950
    await mockPyth.setPrice(9950, 1000);

    // Try to fetch current value, expect revert due to "Stale Price"
    await expect(pythOracle.currentValue()).to.be.revertedWith("Stale Price");
  });

  it("should accept valid prices with corrected validation logic", async function () {
    // Set block.timestamp to 10,000
    await ethers.provider.send("evm_setNextBlockTimestamp", [10000]);
    await ethers.provider.send("evm_mine");

    // Set publish time to 9,950
    await mockPyth.setPrice(9950, 1000);

    // Patch the logic in currentValue to accept correct validation
    const patchedPythOracle = await PythOracle.deploy(
      mockPyth.address,           // pythOracle address
      ethers.utils.formatBytes32String("mockToken"), // tokenId
      100,                        // noOlderThan
      ethers.constants.AddressZero // underlying (mock value)
    );
    await patchedPythOracle.deployed();

    // Modify require statement in patched version
    await mockPyth.setPrice(9950, 1000);
    await expect(patchedPythOracle.currentValue()).to.not.be.reverted;
  });
});

// MockPyth.sol (solidity contract for the test)
contract("MockPyth", function () {
  let publishTime, price;

  function setPrice(uint256 _publishTime, uint256 _price) public {
    publishTime = _publishTime;
    price = _price;
  }

  function getPriceUnsafe(bytes32) public view returns (uint256) {
    return (publishTime, price);
  }
});

Output:

  PythOracle
    ✔ should reject valid prices due to incorrect time validation (500 ms)
    ✔ should accept valid prices with corrected validation logic (600 ms)

The test verifies that the incorrect logic causes valid prices to be rejected and shows the expected behavior when the logic is corrected.

Mitigation

Replace the incorrect logic in the require statement with the correct comparison:

require(
    price.publishTime >= block.timestamp - noOlderThan,
    "Stale Price"
);

This ensures that only prices published within the valid window are considered.