Missing circuit breaker and deviations check in Chainlink price feed

Summary

Chainlink price responses don't have any circuit breaker or deviation check that control the sanity of the prices returned by the oracle.

Vulnerability Detail

The UbiquityPool contract relies on the Chainlink oracle to price collateral assets, which determine the amount of collateral required to mint dollars, or the amount of collateral returned when redeeming dollars.

Chainlink responses are validated as part of the updateChainLinkCollateralPrice() function:

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

As seen in the previous code, there is no check to ensure that answer does not go below or above a certain price.

Chainlink aggregators have a built-in circuit breaker to check if the price of an asset falls within a certain price range. If the used price feeds experience a huge drop or rise in value, the feed will continue to return minimum or maximum values determined by the boundaries. Check https://rekt.news/venus-blizz-rekt/ for a reference of a real incident caused by this issue.

The AccessControlledOffchainAggregator contract (available at https://etherscan.io/address/0x478238a1c8B862498c74D0647329Aef9ea6819Ed#readContract) defines functions minAnswer() and maxAnswer() that provide the boundaries for valid answers.

Moreover, there is no deviation check on the price returned by the oracle. If the new price has a large deviation from the previous known value, this new value will simply be accepted as the current price. Any outlier or reported bad value will abruptly shift the price of the collateral asset. Reference the Silo incident for a real event of this issue: https://twitter.com/SiloFinance/status/1731013330716795038

Impact

Chainlink's circuit breaker impose boundaries on the response price that cap the price returned in response. If the real price of an asset falls outside this range, the oracle will return an incorrect value.

Missing deviation checks will also enable potential outliers in prices to be accepted as valid responses.

Code Snippet

https://github.com/sherlock-audit/2023-12-ubiquity/blob/main/ubiquity-dollar/packages/contracts/src/dollar/libraries/LibUbiquityPool.sol#L543-L552

Tool used

Manual Review

Recommendation

Validate that answer is within the bounds of minAnswer() and maxAnswer().
Introduce a deviation check that controls how much a new answer can deviate from the current known value.
Introduce stricter boundaries for price answers. Aside from the valid range of values provided by Chainlink, introduce tighter ranges for collateral assets.

When any or some of these conditions isn't met, pause the collateral so that it can be manually reviewed by the protocol team.

Duplicate of #144

sherlock-admin2 commented 8 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