DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Valid

Incorrect Price Validation in KeeperProxy

Summary

The KeeperProxy contract validates market prices by comparing data from the PerpetualVault with Chainlink price feeds. However, two critical issues have been identified in its price validation logic:

  1. The long token’s price is incorrectly compared against the index token’s price range instead of using its own price data.

  2. The _check function applies improper decimal scaling, assuming an input precision of 30 decimals, which may not always be accurate.

These flaws could result in inaccurate price validations, potentially allowing outdated or manipulated prices to trigger keeper actions, thereby compromising the protocol's integrity.

Vulnerability Details

  • Incorrect Long Token Price Validation:
    The contract currently checks the long token’s price against the index token’s price range using:

    _check(marketData.longToken, prices.indexTokenPrice.min);
    _check(marketData.longToken, prices.indexTokenPrice.max);

    Issue:
    If the long token differs from the index token, it should be validated against its own price data (e.g., prices.longTokenPrice.min and prices.longTokenPrice.max). Comparing the long token to index token price bounds could misrepresent its actual market value, leading to faulty validations.

  • Decimal Scaling Issue in _check:
    The function normalizes prices using the following logic:

    uint256 decimals = 30 - IERC20Meta(token).decimals();
    price = price / 10 ** (decimals - 8); // Chainlink price decimals is always 8.

    Issue:
    This implementation assumes the input price is always formatted with 30 decimals before adjusting it to match Chainlink's 8-decimal standard. If the input price format deviates from this assumption, the conversion will be inaccurate, leading to incorrect comparisons between on-chain and Chainlink prices. Such discrepancies could permit outdated or manipulated prices to bypass validation.

Impact

  • Inaccurate Price Validation:
    If the long token’s price is assessed using incorrect bounds or if decimal scaling misrepresents actual values, keeper actions might execute trades based on faulty market data, potentially leading to unfavorable transactions.

  • Vulnerability to Price Manipulation:
    Poor validation logic could allow attackers to exploit price mismatches, forcing the protocol to execute trades at artificially manipulated prices, resulting in potential financial losses.

Tools Used

  • Manual Code Review

Recommendations

  1. Ensure Correct Price Data for Long Tokens:

    • Modify the validation logic to reference the long token’s own price range. If applicable, replace:

      _check(marketData.longToken, prices.indexTokenPrice.min);
      _check(marketData.longToken, prices.indexTokenPrice.max);

      with:

      _check(marketData.longToken, prices.longTokenPrice.min);
      _check(marketData.longToken, prices.longTokenPrice.max);
    • Confirm that the long token’s price feed is accurately integrated.

  2. Fix Decimal Scaling Logic:

    • Reevaluate the assumption that all input prices use 30 decimals.

    • Implement a more dynamic normalization method that correctly adjusts prices based on actual token decimals, ensuring proper conversion to Chainlink’s 8-decimal format.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_validatePrice_no_check_for_longTokenPrice

Likelihood: None/Very Low, everytime the keeper send a price via run/runNextAction (sent by the Gamma keeper). Impact: Medium/High, does not check the longTokenPrice, it could go out of range. Keep in mind indexToken == longToken, an error from the keeper could be considered informational.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.