Unhandled chainlink revert would lock price oracle access in `LibUbiquityPool.updateChainLinkCollateralPrice()`

[0xRizwan]

Title

Unhandled chainlink revert would lock price oracle access in LibUbiquityPool.updateChainLinkCollateralPrice()

Severity

Medium

Vulnerability details

    function updateChainLinkCollateralPrice(uint256 collateralIndex) internal {
        UbiquityPoolStorage storage poolStorage = ubiquityPoolStorage();

        AggregatorV3Interface priceFeed = AggregatorV3Interface(
            poolStorage.collateralPriceFeedAddresses[collateralIndex]
        );

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

        ) = // answeredInRound
            priceFeed.latestRoundData();

        // fetch number of decimals in chainlink feed
        uint256 priceFeedDecimals = priceFeed.decimals();

        // validation
        require(answer > 0, "Invalid price");
        require(
            block.timestamp - updatedAt <
                poolStorage.collateralPriceFeedStalenessThresholds[
                    collateralIndex
                ],
            "Stale data"
        );

        // convert chainlink price to 6 decimals
        uint256 price = uint256(answer).mul(UBIQUITY_POOL_PRICE_PRECISION).div(
            10 ** priceFeedDecimals
        );

        poolStorage.collateralPrices[collateralIndex] = price;

        emit CollateralPriceSet(collateralIndex, price);
    }

updateChainLinkCollateralPrice() 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.

Onchain updates take place at the smart contract level, where a multi-signature safe (multisig) is used to modify onchain parameters relating to a Chainlink service. This can include replacing faulty nodes on a specific oracle network, introducing new features such as Offchain Reporting, or resolving a smart contract logic error. The multisig-coordinated upgradability of Chainlink services involves time-tested processes that balance collusion-resistance with the flexibility required to implement improvements and adjust parameters. Reference link

Impact

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

Recommendation to fix

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.

For example:

function getPrice(address priceFeedAddress) external view returns (int256) {
        try AggregatorV3Interface(priceFeedAddress).latestRoundData() returns (
            uint80,         // roundID
            int256 price,   // price
            uint256,        // startedAt
            uint256,        // timestamp
            uint80          // answeredInRound
        ) {
            return price;
        } catch Error(string memory) {            
            // handle failure here:
            // revert, call propietary fallback oracle, fetch from another 3rd-party oracle, etc.
        }
    }

References

1)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.

2)Reference news article: https://cryptonews.net/news/defi/20502745/

[rndquu]

Thank you for filing an issue

We already have a separate issue for better chainlink oracle (with the fallback one)

[ubiquibot[bot]]

# Issue was not closed as completed. Skipping.