ZdravkoHr. - Missing minAnswer & maxAnswers checks in ChainlinkPriceFeeds

[sherlock-admin2]

ZdravkoHr.

medium

Missing minAnswer & maxAnswers checks in ChainlinkPriceFeeds

Summary

It's possible that a Chainlink feed will report wrong price in some cases. Chainlink feeds have in-built minAnswer and maxAnswer. If the price to return is outside the range [minAnswer; maxAnswer], the corresponding cap values will be returned. The ChainlinkPriceFeeds:_validatePriceFeedResult() does not validate that the returned value is in the above range.

Vulnerability Detail

The price of this token can either fall below the minAnswer or go beyond the maxAnswer. Imagine in the scenario of a crash of one of the assets that back OHM and its price drops close to 0. This would cause the price of OHM to spike beyond the RBS cushion. A market with treasury funds will be allocated to try to normalize the price, even though the asset is now not reliable. This will allow users to buy OHM at a very cheap price, messing up the whole purpose of the RBS.

Impact

RBS not working correctly, can lead to buying cheaper OHM.

Code Snippet

    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
            );
    }

Tool used

Manual Review

Recommendation

Revert if the price is outside the safe range.

    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 < feed.minAnswer() || roundData.priceInt > feed.maxAnswer()) 
+         revert(); // Revert with the desired custom error
    }

[nevillehuang]

Invalid, most minimum and maximum values are no longer used in most feeds as seen here

In fact, there is a minimum value check already decided by the protocol, that is as long as price returned by chainlink price feed is not less than or equal to zero seen here, olympus allows price to continue to be retrieved. So all this issues are purely design suggestions to implement an appropriate min/max value.