Return of Wrong Oracle version due to Error in validating Timestamp

Summary

The validation at L73 of Oracle.sol contract shows that the loop on Oracle Timestamp is treated like it is in Descending Order instead of ascending order which will allow return of wrong oracle version due to Error in this validation

Vulnerability Detail

 function at(uint256 timestamp) public view returns (OracleVersion memory atVersion) {
        if (timestamp == 0) return atVersion;
        IOracleProvider provider = oracles[global.current].provider;
        for (uint256 i = global.current - 1; i > 0; i--) {
>>>>   if (timestamp > uint256(oracles[i].timestamp)) break;
            provider = oracles[i].provider;
        }
        return provider.at(timestamp);
    }

In a simple simplified form : let us assume timestamp(t) = 5 oracles timestamp array(Ot) = [2,4,5,6,8,10] based on the validation used in code base it will always be Ot[0} insatead of Ot[2]

Impact

Return of wrong oracle version which will affect proper functionality of code base

Code Snippet

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L73

 function at(uint256 timestamp) public view returns (OracleVersion memory atVersion) {
        if (timestamp == 0) return atVersion;
        IOracleProvider provider = oracles[global.current].provider;
        for (uint256 i = global.current - 1; i > 0; i--) {
>>>>   if (timestamp > uint256(oracles[i].timestamp)) break;
            provider = oracles[i].provider;
        }
        return provider.at(timestamp);
    }

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L44

   function request(address account) external onlyAuthorized {
       ...

>>>>        oracles[global.current].timestamp = uint96(currentTimestamp);
        _updateLatest(latestVersion);
    }

https://github.com/sherlock-audit/2023-09-perennial/blob/main/perennial-v2/packages/perennial-oracle/contracts/Oracle.sol#L89

      function _updateCurrent(IOracleProvider newProvider) private {
    ...
        if (global.current != 0) {
            OracleVersion memory latestVersion = oracles[global.current].provider.latest();
            if (latestVersion.timestamp > oracles[global.current].timestamp)
 >>>>         oracles[global.current].timestamp = uint96(latestVersion.timestamp);
        }
  ...
    }

as seen above in the _updateCurrent(...) & request(...) functions above oracles[global.current].timestamp is updated in ascending order based on Timestamp since time moves in increasing progression

Tool used

Foundry, Manual Review

Recommendation

 function at(uint256 timestamp) public view returns (OracleVersion memory atVersion) {
        if (timestamp == 0) return atVersion;
        IOracleProvider provider = oracles[global.current].provider;
        for (uint256 i = global.current - 1; i > 0; i--) {
+++    if ( uint256(oracles[i].timestamp) > timestamp ) break;
---  if (timestamp > uint256(oracles[i].timestamp)) break;
            provider = oracles[i].provider;
        }
        return provider.at(timestamp);
    }

as seen above it should be "uint256(oracles[i].timestamp) > timestamp" and "timestamp < uint256(oracles[i].timestamp)"

sherlock-audit / 2023-09-perennial-judging

Topmark - Return of Wrong Oracle version due to Error in validating Timestamp #57