bitsurfer - Perennial `oracleVersion` increasing monotonic, while the Chainlink roundId may not be monotonic

[sherlock-admin]

bitsurfer

medium

Perennial oracleVersion increasing monotonic, while the Chainlink roundId may not be monotonic

Summary

Perennial oracleVersion increasing monotonic, while the Chainlink roundId may not be monotonic

Vulnerability Detail

The Perennial protocol relies on storing historical data and fetching it from Chainlink using an identifier called oracleVersion. This oracleVersion is assumed to increase monotonically by 1, similar to the roundId in Chainlink. However, it has been discovered that this assumption is incorrect.

The issue arises from the fact that the oracleVersion in Perennial does not consistently increase by 1 for each new round of data fetched from Chainlink. This can lead to incorrect data associations and mismatches between the stored historical data and the corresponding Chainlink rounds.

According to Chainlink documentation, https://docs.chain.link/data-feeds/historical-data#roundid-in-aggregator-aggregatorroundid

Data feeds are updated in rounds. Rounds are identified by their roundId, which increases with each new round. This increase may not be monotonic.

Here it mention about, the roundId increases may not be monotonic (increase).

Meanwhile, if we look at perennial's code, the settleVersion is increasing the oracleVersion (roundId) with 1 (for next settle txn)

File: PrePosition.sol
133:     function settleVersion(PrePosition storage self, uint256 currentVersion) internal view returns (uint256) {
134:         uint256 _oracleVersion = self.oracleVersion;
135:         return _oracleVersion == 0 ? currentVersion : _oracleVersion + 1;
136:     }

File: Product.sol
95:         // Get settle oracle version
96:         uint256 _settleVersion = _position.pre.settleVersion(currentOracleVersion.version);
97:         IOracleProvider.OracleVersion memory settleOracleVersion = _settleVersion == currentOracleVersion.version
98:             ? currentOracleVersion // if b == c, don't re-call provider for oracle version
99:             : atVersion(_settleVersion);

In cases where the pre-settle version does not match the current oracleVersion, the Perennial protocol will fetch the data using the atVersion function. This involves steps of loading, building data and fetching the Chainlink getRoundData with the roundId corresponding to the oracleVersion of the pre-settle version.

File: UPayoffProvider.sol
53:     function atVersion(uint256 oracleVersion) public view returns (IOracleProvider.OracleVersion memory) {
54:         return _transform(oracle().atVersion(oracleVersion));
55:     }

File: ChainlinkOracle.sol
094:     function atVersion(uint256 version) public view returns (OracleVersion memory oracleVersion) {
095:         return _buildOracleVersion(registry.getRound(base, quote, _versionToRoundId(version)), version);
096:     }
097:
...
104:     function _buildOracleVersion(ChainlinkRound memory round) private view returns (OracleVersion memory) {
105:         Phase memory phase = _phases[round.phaseId()];
106:         uint256 version = uint256(phase.startingVersion) + round.roundId - uint256(phase.startingRoundId);
107:         return _buildOracleVersion(round, version);
108:     }
...
116:     function _buildOracleVersion(ChainlinkRound memory round, uint256 version)
117:     private view returns (OracleVersion memory) {
118:         Fixed18 price = Fixed18Lib.ratio(round.answer, _decimalOffset);
119:         return OracleVersion({ version: version, timestamp: round.timestamp, price: price });
120:     }

File: ChainlinkRegistry.sol
48:     function getRound(ChainlinkRegistry self, address base, address quote, uint256 roundId) internal view returns (ChainlinkRound memory) {
49:         (, int256 answer, , uint256 updatedAt, ) =
50:             FeedRegistryInterface(ChainlinkRegistry.unwrap(self)).getRoundData(base, quote, uint80(roundId));
51:         return ChainlinkRound({roundId: roundId, timestamp: updatedAt, answer: answer});
52:     }

Impact

Perennial failed to fetch data from Chainlink when roundId is not monotonic increase

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/interfaces/types/PrePosition.sol#L133-L136

Tool used

Manual Review

Recommendation

Implement a more reliable tracking mechanism for the oracleVersion, not just assuming it will increase +1 every roundId. Or utilizing a different identifier that ensures the correct association between stored historical data and Chainlink rounds.

[arjun-io]

Although Chainlink's documentation is not clear here, the assumption is that the roundId within a phase is monotonically increasing and that non-monotonic increases only happen when the phase is updated.

This can be seen in the Chainlink aggregator cod here: https://etherscan.io/address/0xE62B71cf983019BFf55bC83B48601ce8419650CC#code#F8#L682 (note the ++ usage)

Perennial's oracles are designed to handle the non-monotonic increase when the phase changes.