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

Incorrect Price Validation in KeeperProxy

Summary

The KeeperProxy contract performs price validations by comparing market prices obtained from the PerpetualVault with Chainlink data feeds. Two key issues have been identified in its price validation logic:

  1. The long token's price is checked against the index token’s price bounds rather than using its own price data.

  2. The decimal scaling in the _check function incorrectly normalizes prices, assuming an input precision of 30 decimals, which may not always be the case.

These issues can lead to inaccurate price validations, potentially allowing stale or manipulated prices to trigger keeper actions, thereby undermining the protocol’s security.

Vulnerability Details

  • Long Token Price Validation:
    The contract currently validates the long token’s price with the following lines:

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

    Issue:
    If the long token is not the same as the index token, it should use its own price data (e.g., prices.longTokenPrice.min and prices.longTokenPrice.max). Using index token prices for the long token can result in misjudging the actual market value, leading to incorrect validations.

  • Decimal Scaling Issue in _check:
    The price normalization logic is as follows:

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

    Issue:
    This logic assumes that the input price has 30 decimals, and then scales it down to 8 decimals. If the input price does not adhere to a 30-decimal format, the conversion will be inaccurate, leading to an incorrect comparison between the on-chain price and the Chainlink feed. Such mis-scaling may allow stale or manipulated prices to pass the validation.

Impact

  • Inaccurate Price Validation:
    If the long token's price is validated against incorrect bounds or if the decimal scaling is off, keeper actions may be executed based on erroneous market data. This can lead to orders being triggered at unfavorable prices.

  • Exposure to Price Manipulation:
    Faulty validations could allow an attacker to exploit price discrepancies, potentially forcing the protocol to execute trades at manipulated prices and thereby causing financial loss.

Tools Used

  • Manual Code Review:

Recommendations

  1. Correct Price Data Usage for Long Tokens:

    • Modify the validation logic to use the long token’s own price bounds. For example, if available, 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);
    • Ensure that the long token price feed is integrated correctly.

  2. Revise Decimal Scaling Logic:

    • Reassess the assumption that input prices use 30 decimals.

    • Develop a more flexible normalization method that uses the actual token decimals and properly converts prices to 8-decimal precision, matching the Chainlink standard.

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.

Give us feedback!