0xhacksmithh - DataFeed will use the wrong price if the Chainlink aggregator returns price outside min/max range

[sherlock-admin3]

0xhacksmithh

medium

DataFeed will use the wrong price if the Chainlink aggregator 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.

Here is an real world example of above senario :: Venus on BSC when LUNA imploded

Vulnerability Detail

Note that there is only checks for price to be non-zero and Healthyness check (which has its own problem explained here), but not within an acceptable range.

A similar issue is mentioned here and here

    function _getDataInBase18()
        private
        view
        returns (uint80 roundId, uint256 answer)
    {
        uint8 decimals = aggregator.decimals();
        (uint80 _roundId, int256 _answer, , uint256 updatedAt, ) = aggregator
            .latestRoundData();
        require(_answer > 0, "DF: feed is deprecated");
        require(
            // solhint-disable-next-line not-rely-on-time
            block.timestamp - updatedAt <= _HEALTHY_DIFF,
            "DF: feed is unhealthy"
        );
        roundId = _roundId;
        answer = uint256(_answer).convertToBase18(decimals);
    }

Impact

The wrong price may be returned in the event of a market crash.

Code Snippet

https://github.com/sherlock-audit/2024-05-midas/blob/main/midas-contracts/contracts/feeds/DataFeed.sol#L64-L80

Tool used

Manual Review

Recommendation

        (uint80 _roundId, int256 _answer, , uint256 updatedAt, ) = aggregator
            .latestRoundData();
        require(_answer > 0, "DF: feed is deprecated");
        require(
            // solhint-disable-next-line not-rely-on-time
            block.timestamp - updatedAt <= _HEALTHY_DIFF,
            "DF: feed is unhealthy"
        );
+          require(price >= minPrice && price <= maxPrice, "invalid price"); // @audit use the proper minPrice and maxPrice for asset
        roundId = _roundId;
        answer = uint256(_answer).convertToBase18(decimals);
    }

[0xhacksmithh]

Escalate I don't understand why this is invalid. it's classic bug found in some previous audits and there in report i mentioned and provide links to those Bug report.

[sherlock-admin3]

Escalate I don't understand why this is invalid. it's classic bug found in some previous audits and there in report i mentioned and provide links to those Bug report.

The escalation could not be created because you are not exceeding the escalation threshold.

You can view the required number of additional valid issues/judging contest payouts in your Profile page, in the Sherlock webapp.

[pronobis4]

Because everything is validated off-chain, the severity should be low.