Unhandled chainlink revert would lock all price oracle access.

Summary

A call to the latestRoundData function could potentially revert and make it impossible to query any prices. Feeds cannot be changed after they are configured, so this would result in a permanent denial of service.

Vulnerability Detail

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.

Refer to this guidance for more information regarding potential risks to account for when relying on external price feed providers.

Impact

If a configured Oracle feed has malfunctioned or ceased operating, but the smart contract does not have any alternative data source, nor does the contract allow updates to data sources, that contract will be permanently bricked.

This would be bad for any protocols and all other protocol's functionality that rely on the price feed will be non-responsive due to calls to the price oracles reverting.

Code Snippet

        // fetch latest price
        (
            ,
            // roundId
            int256 answer, // startedAt
            ,
            uint256 updatedAt,

        ) = // answeredInRound
            priceFeed.latestRoundData();

Reference

Tool used

Manual Review

Recommendation

Surround the call to the latestRoundData function 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.

Another Valid Case

A probable change that would be required is something similar to,

+ try{
        // fetch latest price
        (
            ,
            // roundId
            int256 answer, // startedAt
            ,
            uint256 updatedAt,

        ) = // answeredInRound
            priceFeed.latestRoundData();
+ } catch 
+ {
+   << case when latestRoundData function can't be queried to >>
+ }

Duplicate of #110

sherlock-admin2 commented 7 months ago

1 comment(s) were left on this issue during the judging contest.

auditsea commented:

The issue describes about not checking min/max value from Chainlink for data staleness, but this is not required