ChainlinkOracleV1 Manual Review Findings

COV-01M: Improper Integration of Chainlink Oracles

Type	Severity	Location
External Call Validation		ChainlinkOracleV1.sol:L28, L31

Description:

The referenced invocation of latestRoundData is insecure as it does not properly sanitize the result of the oracle call.

Impact:

Currently, a misbehaving Chainlink oracle will not be detected by the 0xPhase system causing it to consume incorrect or outdated / stale price points for the assets it is querying.

Example:

Copy
18/// @inheritdoc IOracle
19function getPrice(
20  address asset
21) external view override returns (uint256 price) {
22  require(
23    priceFeeds[asset] != address(0),
24    "ChainlinkOracleV1: Price feed does not exist"
25  );
26
27  IAggregator aggregator = IAggregator(priceFeeds[asset]);
28  (, int256 itemPrice, , , ) = aggregator.latestRoundData();
29  int256 scaled = MathLib.scaleAmount(itemPrice, aggregator.decimals(), 18);
30
31  price = MathLib.onlyPositive(scaled);
32}

Recommendation:

We advise the code to be updated, enforcing proper sanitization measure(s) to the external Chainlink oracle call.

The data point of interest the latestRoundData function yields is the updatedAt timestamp. The desire is to enforce a particular "heartbeat" of data validity that ensures the updatedAt value satisfies the time threshold imposed by the 0xPhase protocol. We should note that Chainlink imposes different heartbeats for different asset types and as such the time limit that should be imposed needs to be sensible based on the 0xPhase protocol's needs and the idle-time threshold Chainlink has set for each particular data feed.

As an alternative, we advise an administrative manual "pause" mechanism to be introduced, preventing price measurements from the ChainlinkOracleV1::getPrice method to be utilized. This will permit the 0xPhase team to be able to quickly react in case of abnormal market events such as that of the LUNA price crash.

Alleviation (3dd3d7bf0c2693b2f9c23bacedfa420393f7ea84):

An oracle heartbeat has been imposed per price-feed in the updated codebase, alleviating this exhibit in full and ensuring that oracle data points are utilized as long as they are not stale.