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

10 stars 9 forks source link

Inadequate price validation in `getSqrtPrice()` results in usage of stale prices. #297

Closed howlbot-integration[bot] closed 4 months ago

howlbot-integration[bot] commented 4 months ago

Lines of code

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

Vulnerability details

Impact

The getSqrtPrice() function does not sufficiently validate the price returned from chainlink's latestRoundData() as it only ensures that the price returned is a positive non-zero number but does not check for staleness of the feed.

During feed staleness, the value returned is > 0 (passing the check) but will remain the same even if the actual price has altered leading to incorrect prices being returned.

Proof of Concept

getSqrtPrice()

    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
        (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
        //...
        require(quoteAnswer > 0 && basePrice.price > 0);

        //...
    }

This function only queries the answer(quoteAnswer) from the latestRoundData(). As such, there is no way to perform checks concerning the validity of the price returned by the oracle.

Due to this insufficient validation, price returned can be stale resulting in the protocol consuming such prices for various operations that could cause issues.

Tools Used

Manual Review

Recommended Mitigation Steps

Validate data feed sufficiently:

    // @audit The threshold for the age of the price data (set as required)
+   uint256 public priceAgeThreshold = 2 hours;

    // @audit Updates the threshold for the age of the price data
+   function updatePriceAgeThreshold(uint256 _priceAgeThreshold ) external onlyOwner {
+       if (_priceAgeThreshold <= 1 hours || _priceAgeThreshold >= 10 hours) { // @audit Set your own as per requirement
+           revert Oracle_INVALID_INPUT();
+       }
+       priceAgeThreshold = _priceAgeThreshold ;
+   }

    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
        // @audit First fetch the data
-       (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
+       (, int256 quoteAnswer,, uint256 updatedAt,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
        //...

+       if (block.timestamp - updatedAt > chainlinkPriceAgeThreshold) {
+           revert Oracle_DATA_OUT_OF_DATE();
+       }
        require(quoteAnswer > 0 && basePrice.price > 0);

        //...
    }

Assessed type

Oracle

c4-judge commented 4 months ago

alex-ppg marked the issue as satisfactory