Submission Details

#1 Chainlink price feed is not sufficiently validated and can return stale price

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L207

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L218

Summary

The Chainlink price feed lacks adequate validation, potentially resulting in the retrieval of stale or outdated price data.

Vulnerability Details

208 (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
219 (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData(); 

There is no validation to check if the answer (or price) received was actually a stale one. Reasons for a price feed to stop updating are listed here(https://ethereum.stackexchange.com/questions/133242/how-future-resilient-is-a-chainlink-price-feed/133843#133843). Using a stale price in the application can result in wrong calculations.

https://docs.chain.link/data-feeds/api-reference

Impact

The latestRoundData fetches the asset price from a Chainlink aggregator using the latestRoundData function. However, there are no checks if the price is stale and valid.

According to Chainlink's documentation, This function does not error if no answer has been reached but returns 0, causing an incorrect price fed. The external Chainlink oracle, which provides index price information to the system, introduces risk inherent to any dependency on third-party data sources. For example, the oracle could fall behind or otherwise fail to be maintained, resulting in outdated data being fed to the index price calculations of the liquidity.

Tools Used

Manual Review

Recommendations

- (,int256 priceEurUsd,,,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();
+(,int256 priceEurUsd,, uint256 updatedAt,) = Chainlink.AggregatorV3Interface(eurUsd).latestRoundData();

+ if (priceEurUsd <= 0) {
+    revert("invalid price data");
+ }

+ if (updatedAt < block.timestamp - 60 * 60 /* 1 hour */) {
+   revert("Stale Price");
+}



- (,int256 assetPriceUsd,,,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();
+(,int256 assetPriceUsd,, uint256 updatedAssetAt,) = Chainlink.AggregatorV3Interface(asset.token.clAddr).latestRoundData();

+ if (assetPriceUsd <= 0) {
+    revert("invalid price data");
+ }

+ if (updatedAssetAt < block.timestamp - 60 * 60 /* 1 hour */) {
+   revert("Stale Price");
+}

Comments and Activity

Lead Judging Started

hrishibhat Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Chainlink-price

hrishibhat Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Known issue

After considering with the protocol team and Codehawks internal team, based on the information provided in the contest page about chainlink prices expected to be accurate and the price calculator contract being out of scope. Considering this all chainlink price validation issues as known issue

Assigned finding tags:

Chainlink-price