sherlock-audit / 2023-07-perennial-judging

2 stars 1 forks source link

minhtrng - PythOracle pushes wrong timestamp when data is requested #167

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

minhtrng

high

PythOracle pushes wrong timestamp when data is requested

Summary

The PythOracle uses wrong timestamps when data is requested, causing no data to be accessible when needed.

Vulnerability Detail

PythOracle.requests pushes current block.timestamp when data is requested:

versionList.push(block.timestamp); 

This will cause the keeper to later record prices for this specific timestamp in PythOracle.commitRequested:

uint256 versionToCommit = versionList[versionIndex];
...
_recordPrice(versionToCommit, pythPrice);

However the protocol operates with timestamps that are divisible by a _granularity defined in PythFactory:

/// @notice Returns the current timestamp
function current() public view returns (uint256) {
    uint256 effectiveGranularity = block.timestamp <= uint256(_granularity.effectiveAfter) ?
        uint256(_granularity.latestGranularity) :
        uint256(_granularity.currentGranularity);

    return Math.ceilDiv(block.timestamp, effectiveGranularity) * effectiveGranularity;
}

So, when the price/OracleVersion at current() timestamp is later on requested in PythOracle.at, it will not be accessible:

function at(uint256 timestamp) public view returns (OracleVersion memory oracleVersion) {
    Fixed6 price = _prices[timestamp];
    return OracleVersion(timestamp, price, !price.isZero());
}

Impact

Prices not accessible due to timestamp mismatch

Code Snippet

https://github.com/sherlock-audit/2023-07-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/pyth/PythOracle.sol#L79

Tool used

Manual Review

Recommendation

Push the current() timestamp instead of block.timestamp

Duplicate of #42

sherlock-admin commented 1 year ago

2 comment(s) were left on this issue during the judging contest.

141345 commented:

d

panprog commented:

should be low or medium because no funds loss is shown