sherlock-audit / 2023-11-olympus-judging

9 stars 7 forks source link

ge6a - ChainlinkPriceFeeds will use a wrong price if the Chainlink registry returns price outside min/max range #180

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

ge6a

medium

ChainlinkPriceFeeds will use a wrong price if the Chainlink registry returns price outside min/max range

Summary

Chainlink aggregators have a built in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset. This would affect the work of the RBS module in different ways. For example, when swapping OHM for this asset, losses would be incurred for the protocol because this asset would be bought at a higher price than the actual one.

Vulnerability Detail

Note there are only checks for 0 price and stale price.

    function _validatePriceFeedResult(
        AggregatorV2V3Interface feed_,
        FeedRoundData memory roundData,
        uint256 blockTimestamp,
        uint256 paramsUpdateThreshold
    ) internal pure {
        if (roundData.priceInt <= 0)
            revert Chainlink_FeedPriceInvalid(address(feed_), roundData.priceInt);

        if (roundData.updatedAt < blockTimestamp - paramsUpdateThreshold)
            revert Chainlink_FeedRoundStale(
                address(feed_),
                roundData.updatedAt,
                blockTimestamp - paramsUpdateThreshold
            );

        if (roundData.answeredInRound != roundData.roundId)
            revert Chainlink_FeedRoundMismatch(
                address(feed_),
                roundData.roundId,
                roundData.answeredInRound
            );
    }

Impact

When the described circumstances arise, the PRICE module would send the wrong price to the RBS module, leading to an inaccurate assessment of the asset and, consequently, financial losses for the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/ChainlinkPriceFeeds.sol#L142-L164

Tool used

Manual Review

Recommendation

Implement the proper check for each asset. It must revert in the case of bad price.

    function _validatePriceFeedResult(
        AggregatorV2V3Interface feed_,
        FeedRoundData memory roundData,
        uint256 blockTimestamp,
        uint256 paramsUpdateThreshold
    ) internal pure {
        if (roundData.priceInt <= 0)
            revert Chainlink_FeedPriceInvalid(address(feed_), roundData.priceInt);

        if (roundData.updatedAt < blockTimestamp - paramsUpdateThreshold)
            revert Chainlink_FeedRoundStale(
                address(feed_),
                roundData.updatedAt,
                blockTimestamp - paramsUpdateThreshold
            );

        if (roundData.answeredInRound != roundData.roundId)
            revert Chainlink_FeedRoundMismatch(
                address(feed_),
                roundData.roundId,
                roundData.answeredInRound
            );

       if (roundData.priceInt < minPrice || roundData.priceInt > maxPrice); // @audit use the proper minPrice and maxPrice for each asset
       {
           revert Chainlink_FeedPriceInvalid(address(feed_), roundData.priceInt);
       }
    }

Duplicate of #42