ChainLinkOracle Manual Review Findings

CLO-01M: Inexplicable Capability of Replacement

Type	Severity	Location
Centralization Concern		ChainLinkOracle.sol:L12-L19

Description:

Chainlink oracle addresses are not expected to change as the AggregatorV3 implementations are meant to be "upgraded" to point to new oracles that Chainlink deploys.

Example:

Copy
12/**
13  * Add mapping for ChainLink price feed for token
14  * @param _token Token address
15  * @param _feed ChainLink price feed address
16  */
17function addTokenPriceFeed(address _token, address _feed) external onlyOwner {
18  tokenToPriceFeed[_token] = _feed;
19}

Recommendation:

We advise the ChainLinkOracle::addTokenPriceFeed function to prevent resetting a particular tokenToPriceFeed entry via a corresponding require check.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The ChainLinkOracle::addTokenPriceFeed no longer permits a price feed to be replaced via a require check evaluating that the existing feed is unset.

CLO-02M: Inexistent Sanitization of Reported Answer

Type	Severity	Location
Mathematical Operations		ChainLinkOracle.sol:L35

Description:

Per the Chainlink guidelines, reported answers need to be validated as strictly positive before being utilized.

Impact:

The likelihood of a negative or zero price is low and as such, the severity of this exhibit is considered "minor".

Example:

Copy
32(, int256 answer, , , ) = AggregatorV3Interface(tokenToPriceFeed[_token]).latestRoundData();
33decimals = uint256(AggregatorV3Interface(tokenToPriceFeed[_token]).decimals());
34
35price = uint256(answer) * 1e18 / (10 ** decimals);

Recommendation:

We advise the code to properly ensure that the answer is greater-than 0 as currently it may perform an unsafe casting operation of a negative number and will generally consume an incorrect price point of 0.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The reported answer of a Chainlink query is validated via the new ChainLinkOracle::_badChainlinkResponse function, guaranteeing that the reported answer is positive and alleviating this exhibit.

CLO-03M: Improper Integration of Chainlink Oracles

Type	Severity	Location
External Call Validation		ChainLinkOracle.sol:L32

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 Steadefi system causing it to consume incorrect or outdated / stale price points for the assets it is querying.

Example:

Copy
21/**
22  * Get token price from ChainLink feed
23  * @param _token Token address
24  * @return price Asset price; expressed in 1e18
25  */
26function consult(address _token) external view returns (uint256) {
27  require(tokenToPriceFeed[_token] != address(0), "No price feed available for this token");
28
29  uint256 price;
30  uint256 decimals;
31
32  (, int256 answer, , , ) = AggregatorV3Interface(tokenToPriceFeed[_token]).latestRoundData();
33  decimals = uint256(AggregatorV3Interface(tokenToPriceFeed[_token]).decimals());
34
35  price = uint256(answer) * 1e18 / (10 ** decimals);
36
37  return price;
38}

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 Steadefi 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 Steadefi 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 ChainLinkOracle::consult method to be utilized. This will permit the Steadefi team to be able to quickly react in case of abnormal market events such as that of the LUNA price crash.

Alleviation (4325253d6de0ea91c1e9fb9e01d2e7e98f3d83a9):

The code of the ChainLinkOracle was significantly revamped akin to the Liquity oracle implementation, enforcing a maximum deviation between reported prices as well as a maximum delay that can elapse until a price report is considered stale. To ensure manual intervention is also possible, an emergency pause mechanism was introduced via the Pausable implementation of OpenZeppelin. As a combination of those actions, we consider this exhibit fully alleviated as the system is equipped to properly handle misbehaviours in the Chainlink oracle.