missing check for the max/min price in the `chainlinkOracle.sol` contract

[code423n4]

Lines of code

https://github.com/code-423n4/2023-07-moonwell/blob/fced18035107a345c31c9a9497d0da09105df4df/src/core/Oracles/ChainlinkOracle.sol#L97-L113

Vulnerability details

Impact

the chainlinkOracle.sol contract specially the getChainlinkPrice function using the aggregator v2 and v3 to get/call the latestRoundData. the function should check for the min and max amount return to prevent some case happen, something like this:

https://solodit.xyz/issues/missing-checks-for-chainlink-oracle-spearbit-connext-pdf https://solodit.xyz/issues/m-16-chainlinkadapteroracle-will-return-the-wrong-price-for-asset-if-underlying-aggregator-hits-minanswer-sherlock-blueberry-blueberry-git

if case like luna happen then the oracle will return the minimum price and not the crashed price.

Proof of Concept

the function getChainlinkPrice:

function getChainlinkPrice(
        AggregatorV3Interface feed
    ) internal view returns (uint256) {
        (, int256 answer, , uint256 updatedAt, ) = AggregatorV3Interface(feed)
            .latestRoundData();
        require(answer > 0, "Chainlink price cannot be lower than 0");
        require(updatedAt != 0, "Round is in incompleted state");

        // Chainlink USD-denominated feeds store answers at 8 decimals
        uint256 decimalDelta = uint256(18).sub(feed.decimals());
        // Ensure that we don't multiply the result by 0
        if (decimalDelta > 0) {
            return uint256(answer).mul(10 ** decimalDelta);
        } else {
            return uint256(answer);
        }
    }

the function did not check for the min and max price.

Tools Used

manual review

Recommended Mitigation Steps

some check like this can be added to avoid returning of the min price or the max price in case of the price crashes.

          require(answer < _maxPrice, "Upper price bound breached");
          require(answer > _minPrice, "Lower price bound breached");

Assessed type

Oracle

[0xSorryNotSorry]

The implementation does not set a min/max value by design. Also Chainlink does not return min/max price as per the AggregatorV3 docs HERE contrary to the reported below;

ChainlinkAggregators have minPrice and maxPrice circuit breakers built into them.

Further proof required as per the context.

[c4-pre-sort]

0xSorryNotSorry marked the issue as low quality report

[alcueca]

The warden is actually right. It is a bit difficult to find, but the minAnswer and maxAnswer can be retrieved from the Chainlink Aggregator, one step through the proxy. A circuit breaker should be implemented on the Moonwell oracle so that when the price edges close to minAnswer or maxAnswer it starts reverting, to avoid consuming stale prices when Chainlink freezes.

[c4-judge]

alcueca marked the issue as satisfactory

[c4-judge]

alcueca marked the issue as primary issue

[c4-judge]

alcueca marked the issue as selected for report