Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chainlink Oracle priceFeed Data May Return Stale Prices #243

Closed
howlbot-integration bot opened this issue Jun 17, 2024 · 1 comment
Closed

Chainlink Oracle priceFeed Data May Return Stale Prices #243

howlbot-integration bot opened this issue Jun 17, 2024 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-69 edited-by-warden 🤖_91_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/main/src/PriceFeed.sol#L44-L58

Vulnerability details

Impact

The PriceFeed contract does not sufficiently validate the Chainlink oracle data feed for stale prices. The failure to validate the freshness of the price may result in the usage of stale prices, leading to incorrect calculations where price matters.

Proof of Concept

In the PriceFeed contract, the getSqrtPrice() function retrieves the price of Quote Token using Chainlink's latestRoundData function, without validating the freshness of the returned price. It simply takes the price from the returned data, ignoring other returned parameters such as updatedAt and roundId.
The unverified return price is used to calculate the square root price of the base token, which is calculated in terms of the quote token in #L54 of the code snippet below.

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

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

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

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

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

        sqrtPrice = FixedPointMathLib.sqrt(price);
    }

The sqrtPricesqrtPricereturn price is used for slippage checking in theSlippageLib.sol#checkPrice()` function.

    function checkPrice(
        uint256 sqrtBasePrice,
        IPredyPool.TradeResult memory tradeResult,
        uint256 slippageTolerance,
        uint256 maxAcceptableSqrtPriceRange
    ) internal pure {
27:     uint256 basePrice = Math.calSqrtPriceToPrice(sqrtBasePrice);

        if (tradeResult.averagePrice == 0) {
            revert InvalidAveragePrice();
        }

33:        if (tradeResult.averagePrice > 0) {
            // short
35:         if (basePrice.lower(slippageTolerance) > uint256(tradeResult.averagePrice)) {
36:             revert SlippageTooLarge();
37:         }
38:     } else if (tradeResult.averagePrice < 0) {
            // long
40:         if (basePrice.upper(slippageTolerance) < uint256(-tradeResult.averagePrice)) {
41:             revert SlippageTooLarge();
42:         }
43:     }

45:     if (
46:         maxAcceptableSqrtPriceRange > 0
47:             && (
48:                 tradeResult.sqrtPrice < sqrtBasePrice * 1e8 / maxAcceptableSqrtPriceRange
49:                     || sqrtBasePrice * maxAcceptableSqrtPriceRange / 1e8 < tradeResult.sqrtPrice
50:             )
51:     ) {
52:         revert OutOfAcceptablePriceRange();
53:     }
    }

Failure to verify the latestness of prices may result in the use of outdated prices, and slippage checks on trading results may not be performed correctly.
According to Chainlink's documentation, updatedAt parameter can help verify whether the returned answer is fresh or not.

Tools Used

Manual Review

Recommended Mitigation Steps

Please verify whether the price is fresh or stale.

    function getSqrtPrice() external view returns (uint256 sqrtPrice) {
---     (, int256 quoteAnswer,,,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
+++     (, int256 quoteAnswer,,uint256 updatedAt,) = AggregatorV3Interface(_quotePriceFeed).latestRoundData();
+++     require(updatedAt >= block.timestamp - 30 * 60, "stale price feed");

        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);
    }

Assessed type

Oracle

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_91_group AI based duplicate group recommendation bug Something isn't working duplicate-69 edited-by-warden sufficient quality report This report is of sufficient quality labels Jun 17, 2024
howlbot-integration bot added a commit that referenced this issue Jun 17, 2024
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jun 28, 2024
@c4-judge
Copy link
Contributor

alex-ppg marked the issue as satisfactory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-69 edited-by-warden 🤖_91_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant