code-423n4 / 2024-05-predy-findings

10 stars 9 forks source link

Lack of Stale Data Check in `getSqrtPrice` Function Can Lead to Incorrect Price Calculations #246

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/e96f378007e3dc56b184079f0c0e4fe48a72efaa/src/PriceFeed.sol#L46

Vulnerability details

Impact

The getSqrtPrice function in the PriceFeed.sol contract retrieves the latest price data from an oracle to compute the square root of the price. However, the function does not verify whether the retrieved price data is stale. Using stale data can result in incorrect price calculations, which may impact critical functionality reliant on accurate and up-to-date price information, leading to financial losses or erroneous contract behavior.

Proof of Concept

In the PriceFeed.sol contract, the getSqrtPrice function is implemented as follows:

function getSqrtPrice() external view returns (uint256 sqrtPrice) {
  (, int256 quoteAnswer, , , ) = AggregatorV3Interface(_quotePriceFeed)
    .latestRoundData();

  // @audit no check for stale price data.

  IPyth.Price memory basePrice = IPyth(_pyth).getPriceNoOlderThan(
    _priceId,
    VALID_TIME_PERIOD
  );

  require(basePrice.expo == -8, "INVALID_EXP");

  require(quoteAnswer > 0 && basePrice.price > 0);

  uint256 price = (uint256(int256(basePrice.price)) * Constants.Q96) /
    uint256(quoteAnswer);
  price = (price * Constants.Q96) / _decimalsDiff;

  sqrtPrice = FixedPointMathLib.sqrt(price);
}

The function does not check if the quoteAnswer obtained from the oracle is based on stale data. Without this verification, the contract may use outdated prices, leading to incorrect calculations.

Example of Stale Data Impact

If the oracle data is outdated due to network issues, delays, or oracle downtime, the getSqrtPrice function will use old price data, resulting in incorrect price computations. This can affect various functionalities dependent on accurate price feeds, such as asset valuation, trading, and risk management, potentially leading to significant financial losses.

Tools Used

Recommended Mitigation Steps

To ensure the function uses up-to-date price data, implement a check for the updatedAt parameter from the latestRoundData() call and verify that it is within an acceptable time range. Here’s an example of how to refactor the function:

  1. Modify the latestRoundData call to include the updatedAt parameter:

    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
     (, int256 quoteAnswer, , uint256 updatedAt, ) = AggregatorV3Interface(
       _quotePriceFeed
     ).latestRoundData();
    
     // Check for stale price data
     require(
       block.timestamp - updatedAt < VALID_TIME_PERIOD,
       "Stale price data"
     );
    
     IPyth.Price memory basePrice = IPyth(_pyth).getPriceNoOlderThan(
       _priceId,
       VALID_TIME_PERIOD
     );
    
     require(basePrice.expo == -8, "INVALID_EXP");
    
     require(quoteAnswer > 0 && basePrice.price > 0);
    
     uint256 price = (uint256(int256(basePrice.price)) * Constants.Q96) /
       uint256(quoteAnswer);
     price = (price * Constants.Q96) / _decimalsDiff;
    
     sqrtPrice = FixedPointMathLib.sqrt(price);
    }

This change ensures that the price data used in the getSqrtPrice function is current and reliable, preventing the use of stale data and maintaining the integrity of the price calculations.

Assessed type

Oracle

c4-judge commented 2 months ago

alex-ppg marked the issue as satisfactory