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

Decimal Precision Error in Price Validation Causes Complete Denial of Service for Keeper Operations

Summary

A decimal precision error in the KeeperProxy._check() function incorrectly normalizes token prices when comparing against Chainlink oracle prices. This causes the price validation to consistently fail, resulting in a complete denial of service for keeper operations that are essential for processing user orders.

Vulnerability Details

The KeeperProxy._check() function attempts to validate token prices by comparing them with Chainlink oracle prices. However, the decimal normalization logic is flawed:

  1. Input prices are in 30 decimals

  2. Chainlink prices are in 8 decimals

  3. The current code attempts to normalize using:

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

For a token like WETH (18 decimals), this results in:

decimals = 30 - 18 = 12
price = price / 10 ** (12 - 8) = price / 10 ** 4

This leaves the price at 26 decimals instead of the intended 8 decimals, making it significantly larger than the Chainlink price and causing the validation to fail or incorrectly pass (there's a low chance of this happening).

Proof of Concept

Using ETH at $2000 as an example:

// Initial values
price = 2000 * 10^30 // Protocol price (30 decimals)
chainLinkPrice = 2000 * 10^8 // Chainlink price (8 decimals)
// Current incorrect normalization
decimals = 30 - 18 = 12
normalizedPrice = 2000 * 10^30 / 10^4 = 2000 * 10^26
// Price difference check
(2000 * 10^26 - 2000 * 10^8) * 10000 / (2000 * 10^8) ≈ 1 * 10^22
// This massive difference will always exceed any reasonable threshold

Impact Details

  1. All keeper operations requiring price validation (run() and runNextAction()) will consistently fail

  2. User orders cannot be processed as keepers are unable to execute them

  3. This effectively breaks core protocol functionality, as position management becomes impossible

  4. If the validation some how passes, it will lead to alot of price related issues such as wrong colateral, shares, and size delta calculatons which will scramble the protocols accounting and increase the risk exposure on GMX

Recommendations

Fix the decimal normalization by directly converting to 8 decimals:

function _check(address token, uint256 price) internal view {
(, int chainLinkPrice, , uint256 updatedAt, ) = AggregatorV2V3Interface(dataFeed[token]).latestRoundData();
require(updatedAt > block.timestamp - maxTimeWindow[token], "stale price feed");
// Direct conversion from 30 decimals to 8 decimals
uint256 decimals = 30 - 8;
price = price / 10 ** decimals;
require(
_absDiff(price, chainLinkPrice.toUint256()) * BPS / chainLinkPrice.toUint256() < priceDiffThreshold[token],
"price offset too big"
);
}

This ensures prices are properly normalized to 8 decimals before comparison, allowing valid keeper operations to proceed.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

invalid_prices_decimals

GMX github documentation: “Prices stored within the Oracle contract represent the price of one unit of the token using a value with 30 decimals of precision. Representing the prices in this way allows for conversions between token amounts and fiat values to be simplified, e.g. to calculate the fiat value of a given number of tokens the calculation would just be: token amount * oracle price, to calculate the token amount for a fiat value it would be: fiat value / oracle price.” Sponsor confirmed the keeper does the same, so price decimals change in function of the token, to be sure the above rule is true. Example for USDC (6 decimals): Prices will have 24 decimals → 1e6 * 1e24 = 1e30. Just a reminder for some submissions: shortToken == collateralTokens, so the decimals is 1e24 for shortToken prices.

Support

FAQs

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