sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

BLACK-PANDA-REACH - Oracles may return stale prices #216

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

BLACK-PANDA-REACH

medium

Oracles may return stale prices

Summary

Both oracles contracts (ChainlinkOracle and ChainlinkFeedOracle) can return stale prices because of the lack of checks for it.

Vulnerability Detail

After calling latestRoundData at the Chainlink oracles, both contracts make some checks to ensure the validity of the output:

if (uint64(round.roundId) == 0 || round.timestamp == 0) revert InvalidOracleRound();

This is to ensure the roundId is valid but it forgets to check that the price is fresh and not stale.

But the contract is forgetting to ensure that the answer given by the oracle is fresh, meaning the answer given hasn't been updated more than a certain time ago. For ETH/USD oracle, the heartbeat is of 3600 seconds, meaning the oracle must be updated at least every 1 hour.

image

It's a must to check that the answer given by the oracle (ETH/USD in our case) has been updated in the last hour, and no more.

Related report: https://github.com/sherlock-audit/2023-02-blueberry-judging/issues/94

Impact

If the Chainlink oracle is returning stale prices, the protocol will make a version update with the wrong price so that will carry losses to the users in Perennial.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/ChainlinkOracle.sol#L64 https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-oracle/contracts/ChainlinkFeedOracle.sol#L97

Tool used

Manual Review

Recommendation

It's recommended to check the updatedAt value with the current block.timestamp to ensure that the time passed since that price was set is no more than staleFeedThreshold (the recommended for ETH/USD is 1 hour).

if (block.timestamp - round.timestamp > staleFeedThreshold) revert();