medium

Chainlink oracle will return the wrong price if the aggregator hits `minAnswer`

Reward

Total

0.56 USDC

0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
0.01 USDC
Selected Submission

Chainlink oracle will return the wrong price if the aggregator hits minAnswer

Severity

Medium Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-07-foundry-defi-stablecoin/tree/main

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 and vice versa.

Vulnerability Details

The staleCheckLatestRoundData function in OracleLib.sol is only checking for the stale price. But no checks are done to handle that.

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

[21]

There is no function for checking only this as well in the library. The checks are not done in DSCEngine.sol file. There are two instances of that:

        (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();

[345]

        (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();

[363]

Impact

This would allow users to continue mintDsc, burnDsc etc. but at the wrong price. This is exactly what happened to Venus on BSC when LUNA crashed.

Tools Used

chainlink docs, foundry test and previous audit reports

Recommendations

Consider using the following checks:

(uint80, int256 answer, uint, uint, uint80) = oracle.latestRoundData();

// minPrice check
require(answer > minPrice, "Min price exceeded");
// maxPrice check
require(answer < maxPrice, "Max price exceeded");

Also some gas could be saved when used revert with custom error for doing the check.