Risk of Incorrect Asset Pricing by ChainlinkPriceOracle in Case of Underlying Aggregator Reaching minAnswer

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 is exactly what happened to Venus on BSC when LUNA imploded.

Vulnerability Detail

To fetch the price of the requested tokens, ChainlinkPriceOracle::getTokenPrice() utilize ChainlinkPriceOracle::_getFeedValue() which pulls the associated aggregator and requests round data from it. Chainlink aggregators have minPrice and maxPrice circuit breakers built into them. This means that if the price of the asset drops below the minimum price, the protocol will continue to value the token at the minimum price instead of it's actual value.

Impact

If an asset collapses, like the recent USDC depeg, the protocol will always return the minimum price of token.

Code Snippet

https://github.com/sherlock-audit/2024-02-smilee-finance/blob/main/smilee-v2-contracts/src/providers/chainlink/ChainlinkPriceOracle.sol#L90-L108

Tool used

Manual review

Recommendation

ChainlinkPriceOracle::getTokenPrice() should cross-check the returned answer against the minPrice/maxPrice and revert if the answer is outside of these bounds:

    function getTokenPrice(address token) public view returns (uint256) {
        if (token == address(0)) {
            revert AddressZero();
        }

        address priceFeed = feeds[token].get();
        if (priceFeed == address(0)) {
            revert TokenNotSupported();
        }

        OracleValue memory price = _getFeedValue(priceFeed);

        // Protect against stale feeds:
        if (block.timestamp - price.lastUpdate > getPriceFeedMaxDelay(token)) {
            revert PriceTooOld();
        }

+       if (price.value <= minPrice) revert("lower price bound breached");
+       if (price.value >= maxPrice) revert("upper price bound breached");

        return price.value;
    }

This ensures that a false price will not be returned if the underlying asset's value hits the minPrice.

sherlock-audit / 2024-02-smilee-finance-judging

y0ng0p3 - Risk of Incorrect Asset Pricing by ChainlinkPriceOracle in Case of Underlying Aggregator Reaching minAnswer #129