shealtielanz - `ChainlinkPriceFeeds.sol` will return the wrong price for an asset if underlying aggregator hits `minAnswer` or `maxAnswer` as `_validatePriceFeedResult()` doesn't check against it. which will affect the `RBS`

[sherlock-admin2]

shealtielanz

medium

ChainlinkPriceFeeds.sol will return the wrong price for an asset if underlying aggregator hits minAnswer or maxAnswer as _validatePriceFeedResult() doesn't check against it. which will affect the RBS

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 user to continue trading/borrowing with the asset but at the wrong price(for Cooler Loans), and can distabilized the RBS(Range Bound Stability) system.

• This is similar to what happened to Venus on BSC when LUNA imploded.

  Vulnerability Detail

  ChainlinkPriceFeeds::_getFeedPrice() uses _validatePriceFeedResult() for Chainlink's feed validation.

  //@audit it doesn't check if the price goes out of bounds for the min or max prices.
  function _validatePriceFeedResult(
      AggregatorV2V3Interface feed_,
      FeedRoundData memory roundData,
      uint256 blockTimestamp,
      uint256 paramsUpdateThreshold
  ) internal pure {
      if (roundData.priceInt <= 0)
          revert Chainlink_FeedPriceInvalid(address(feed_), roundData.priceInt);
  
      if (roundData.updatedAt < blockTimestamp - paramsUpdateThreshold)
          revert Chainlink_FeedRoundStale(
              address(feed_),
              roundData.updatedAt,
              blockTimestamp - paramsUpdateThreshold
          );
  
      if (roundData.answeredInRound != roundData.roundId)
          revert Chainlink_FeedRoundMismatch(
              address(feed_),
              roundData.roundId,
              roundData.answeredInRound
          );
  }

  ChainlinkFeedRegistr::latestRoundData pulls the associated aggregator and requests round data from it. ChainlinkAggregators have minPrice and maxPrice circuit breakers built into them. This means that if the price of the asset drops below the minPrice, the protocol will continue to value the token at minPrice instead of it's actual value. This will allow users to take out huge amounts of bad debt and bankrupt the protocol.

Example: TokenA has a minPrice of $1. The price of TokenA drops to $0.10. The aggregator still returns $1 allowing the user to trade/borrow against TokenA as if it is $1 which is 10x it's actual value.

Note: Chainlink oracle is used a just one piece of the Price/Oracle system and it is assumed that using a combination of other oracles, a scenario like this can be avoided. However, this is not the case because the other oracles also have their flaws that can still allow this to be exploited. As an example if the chainlink oracle is being used with a UniswapV3Oracle which uses a long TWAP then this will be exploitable when the TWAP is near the minPrice on the way down.

Impact

ChainlinkPriceFeeds.sol is part of the PRICE Module, and as per Olympus V3 Overview

`PRICE` — Used to store historical price oracle data. Used for the functionality of the Range-Bound Stability (RBS) system.

In the event that an asset crashes (i.e. LUNA) the Range-Bound Stability (RBS) system will be affected and can be manipulated to Trade/loan at an inflated price.

Code Snippet

• https://github.com/sherlock-audit/2023-11-olympus/blob/main/bophades/src/modules/PRICE/submodules/feeds/ChainlinkPriceFeeds.sol#L142C1-L164C6

  Tool used

Manual Review & Solodit

Recommendation

_validatePriceFeedResult() should check the returned answer against the minPrice/maxPrice and revert if the answer is outside of the bounds:

    function _validatePriceFeedResult(
..SNIP..
+   if (roundData.priceInt >= maxPrice || roundData.priceInt <= minPrice) revert();

Duplicate of #42