Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Redundant Price Validation For Same Token in GMX Markets

codertjay

Summary

The KeeperProxy validates both indexToken and longToken prices separately even though they are currently the same
token in GMX markets. This causes unnecessary gas consumption, but could serve as a safety check for future market
implementations.

Vulnerability Details

Current Documentation on the PerpetualVault:
This was stated in there

/**
* For now, we only support the GMX market in which `indexToken` is the same as `longToken`.
* ...
*/

Price Validation:

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// Redundant validations for same token
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min); // Extra gas
_check(marketData.longToken, prices.indexTokenPrice.max); // Extra gas
}

Each _check operation:

function _check(address token, uint256 price) internal view {
(, int256 chainLinkPrice,, uint256 updatedAt,) = AggregatorV2V3Interface(dataFeed[token]).latestRoundData();
require(updatedAt > block.timestamp - maxTimeWindow[token], "stale price feed");
// ... more operations
}

which takes more time and since if token are same and the longToken price is not been used in the Perpetualvault then
this is a redundant check and can be removed to save gas.

Impact

LOW/Gas Optimization because:

  1. Extra gas cost from redundant validations

  2. Current duplication serves as safety check for future markets

  3. No security risk, just efficiency concern

  4. May be intentional for upgrade-proofing

Gas Cost Per Validation:

  • Chainlink Oracle call

  • Storage reads for maxTimeWindow

  • Price conversion operations

  • Multiple require checks

Recommendations

  1. Keep current implementation but use correct price fields:
    This way even if longToken is different then the currect calculation is done

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// Validate each token with its own price field
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
// Use longTokenPrice instead of indexTokenPrice ( for future markets)
_check(marketData.longToken, prices.longTokenPrice.min);
_check(marketData.longToken, prices.longTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
// rest of the code
}

  1. Add flag with proper price fields:
    Add flag to validate longToken separately if still in Gmx market

bool public validateLongTokenSeparately;
​
function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
​
if (validateLongTokenSeparately) {
// Use longTokenPrice for longToken validation
_check(marketData.longToken, prices.longTokenPrice.min);
_check(marketData.longToken, prices.longTokenPrice.max);
}
// rest of the code
}

  1. Add documentation for future upgrades:

/// @notice Validates prices for indexToken and longToken separately
/// @dev Even though currently indexToken == longToken, we validate both with
/// their respective price feeds (indexTokenPrice and longTokenPrice)
/// to support future markets where they might differ
function _validatePrice(address perpVault, MarketPrices memory prices) internal view

This approach:

  1. Maintains separate validations for future-proofing

  2. Uses correct price fields for each token

  3. Allows for future markets where indexToken ≠ longToken

  4. No need to update validation logic during upgrades

The key change is using prices.longTokenPrice instead of prices.indexTokenPrice for longToken validation, making the
code more robust for future markets.

Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Support

FAQs

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