Unhandled chainlink revert would lock price oracle access

[codehawks-bot]

Unhandled chainlink revert would lock price oracle access

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/libraries/OracleLib.sol#L26-L27

Summary

Chainlink's latestRoundData() is used which could potentially revert and make it impossible to query any prices. This could lead to permanent denial of service.

Vulnerability Details

In OracleLib.sol, staleCheckLatestRoundData()

File: src/libraries/OracleLib.sol

21    function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
22        public
23        view
24        returns (uint80, int256, uint256, uint256, uint80)
25    {
26        (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
27            priceFeed.latestRoundData();
28
29        uint256 secondsSince = block.timestamp - updatedAt;
30        if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();
31
32        return (roundId, answer, startedAt, updatedAt, answeredInRound);
33    }

The above functions makes use of Chainlink's latestRoundData() to get the latest price. However, there is no fallback logic to be executed when the access to the Chainlink data feed is denied by Chainlink's multisigs. Chainlink's multisigs can immediately block access to price feeds at will. Therefore, to prevent denial of service scenarios, it is recommended to query Chainlink price feeds using a defensive approach with Solidity’s try/catch structure. In this way, if the call to the price feed fails, the caller contract is still in control and can handle any errors safely and explicitly.

Referring chainlink documentation on how chainlink services are updated. Please note chainlink multisig holds the power of Chainlink’s multisigs can immediately block access to price feeds at will. It should not be taken in wrong way for chainlink though i like chainlink and its product and have respect for them. But this finding is highlighting the possible issue which can happen.

Reference link: https://chain.link/faqs#how-are-chainlink-services-updated

cryptonews article reference:

Reference link: https://cryptonews.net/news/defi/20502745/

Openzeppelin reference: Refer to https://blog.openzeppelin.com/secure-smart-contract-guidelines-the-dangers-of-price-oracles/ for more information regarding potential risks to account for when relying on external price feed providers.

This is similar Medium severity finding found in juicebox audit at code4rena. Reference link:- https://solodit.xyz/issues/m-09-unhandled-chainlink-revert-would-lock-all-price-oracle-access-code4rena-juicebox-juicebox-v2-contest-git

Impact

Call to latestRoundData could potentially revert and make it impossible to query any prices. This could lead to permanent denial of service.

Tools Used

Manual Review

Recommendations

Surround the call to latestRoundData() with try/catch instead of calling it directly. In a scenario where the call reverts, the catch block can be used to call a fallback oracle or handle the error in any other suitable way.

[0xRizwan]

Hi @hans-cyfrin

With due respect to your judging,

In think this issue is a duplicate of #178

Issue tag should be finding-oracle-down

Thank you!

[PatrickAlphaC]

@mohammedrizwann123 agreed