KingNFT - ````Market```` can't get valid price while oracle's ````granularity > 1````

[sherlock-admin]

KingNFT

high

Market can't get valid price while oracle's granularity > 1

Summary

The timestamp used in position is not equal to timestamp used for requesting oracle price. Orders would not be settled in the correct price while PythFactory.effectiveGranularity > 1.

Vulnerability Detail

First, let's see how PythFactory and PythOracle contracts calculate current() timestamp. Please pay attention on L76 of PythFactory.sol, the result is rounded up. For example, if the current block.timestamp = 101 and effectiveGranularity = 100, the calculated result would be 200. Market contract loads this timestamp as context.currentTimestamp (L312 of market.sol:_loadContext()) and uses in positions.

File: perennial-v2\packages\perennial-oracle\contracts\pyth\PythFactory.sol
71:     function current() public view returns (uint256) {
72:         uint256 effectiveGranularity = block.timestamp <= uint256(_granularity.effectiveAfter) ?
73:             uint256(_granularity.latestGranularity) :
74:             uint256(_granularity.currentGranularity);
75: 
76:         return Math.ceilDiv(block.timestamp, effectiveGranularity) * effectiveGranularity;
77:     }

File: perennial-v2\packages\perennial-oracle\contracts\pyth\PythOracle.sol
100:     function current() public view returns (uint256) {
101:         return IPythFactory(address(factory())).current();
102:     }

File: perennial-v2\packages\perennial-oracle\contracts\pyth\PythOracle.sol
86:     function status() external view returns (OracleVersion memory, uint256) {
87:         return (latest(), current());
88:     }

File: perennial-v2\packages\perennial\contracts\Market.sol
574:     function _oracleVersion() private view returns (OracleVersion memory latestVersion, uint256 currentTimestamp) {
575:         (latestVersion, currentTimestamp) = oracle.status();
...
577:     }

File: perennial-v2\packages\perennial\contracts\Market.sol
301:     function _loadContext(address account) private view returns (Context memory context) {
...
310: 
311:         // oracle
312:         (context.latestVersion, context.currentTimestamp) = _oracleVersion();
...
314:     }

Next, we check request() function, please focus on L79 of PythOracle.sol, block.timestamp is pushed to versionList. And the oracle keepers submit price according to block.timestamp.

File: perennial-v2\packages\perennial-oracle\contracts\pyth\PythOracle.sol
77:     function request(address) external onlyAuthorized {
78:         if (versionList.length == 0 || versionList[versionList.length - 1] != block.timestamp) {
79:             versionList.push(block.timestamp);
80:         }
81:     }

File: perennial-v2\packages\perennial-oracle\contracts\Oracle.sol
...
38:         oracles[global.current].provider.request(account);
...
41:     }

File: perennial-v2\packages\perennial\contracts\Market.sol
238:     function _update(
...
246:     ) private {
...
283:         // request version
284:         if (!newOrder.isEmpty()) oracle.request(account);
...
299:     }

While PythFactory.effectiveGranularity > 1, there is a high probability of failing to obtain the price. For example, let's say

block.timestamp = 101
PythFactory.effectiveGranularity = 100

Then

Market.currentTimestamp = 200
oracle.versionList[versionList.length - 1] = 101

Obviously, the Market contract can't query valid price with timestamp = 200.

Impact

The return of a invalid price will trigger the order executing on context.global.latestPrice, which may cause unintended loss to users.

File: perennial-v2\packages\perennial\contracts\Market.sol
582:     function _oracleVersionAt(uint256 timestamp) private view returns (OracleVersion memory oracleVersion) {
583:         oracleVersion = oracle.at(timestamp);
584:         _transform(oracleVersion);
585:     }

File: perennial-v2\packages\perennial\contracts\Market.sol
592:     function _oracleVersionAtPosition(
593:         Context memory context,
594:         Position memory toPosition
595:     ) private view returns (OracleVersion memory oracleVersion) {
596:         oracleVersion = _oracleVersionAt(toPosition.timestamp);
597:         if (!oracleVersion.valid) oracleVersion.price = context.global.latestPrice;
598:     }

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

using current() instead of block.timestamp

File: perennial-v2\packages\perennial-oracle\contracts\pyth\PythOracle.sol
77:     function request(address) external onlyAuthorized {
78:         if (versionList.length == 0 || versionList[versionList.length - 1] != current()) {
79:             versionList.push(current());
80:         }
81:     }

Duplicate of #42

[sherlock-admin]

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

141345 commented:

d