staleCheckLatestRoundData will use the wrong price if Chainlink returns price outside min/max range

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/blob/d1c5501aa79320ca0aeaa73f47f0dbc88c7b77e2/src/libraries/OracleLib.sol#L21

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 allow users to continue using the asset but at the wrong price. This is exactly what happened to Venus on BSC when LUNA imploded - https://rekt.news/venus-blizz-rekt/

Vulnerability Details

There is no min/max price check.

    function staleCheckLatestRoundData(AggregatorV3Interface priceFeed)
        public
        view
        returns (uint80, int256, uint256, uint256, uint80)
    {
        (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound) =
            priceFeed.latestRoundData();

        uint256 secondsSince = block.timestamp - updatedAt;
        if (secondsSince > TIMEOUT) revert OracleLib__StalePrice();

        return (roundId, answer, startedAt, updatedAt, answeredInRound);
    }

Impact

The wrong price may be returned in the event of a market crash. An adversary will then be able to borrow against the wrong price and incur bad debt to the protocol.

Tools Used

Manual review

Recommendations

Add the following checks:

require(answer < maxPrice, "Upper price bound breached");
require(answer > minPrice, "Lower price bound breached");

Cyfrin / 2023-07-foundry-defi-stablecoin