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

Future Compatibility Issue in `_validatePrice` Due to Assumption on `indexToken` and `longToken`

Summary

The _validatePrice function currently assumes that the indexToken and longToken are always the same. This assumption is explicitly mentioned in the comments, stating that

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

This assumption holds true for now but could lead to incorrect validations in the future if these tokens become different. The issue arises because prices.indexTokenPrice is used for both indexToken and longToken, which may not be correct if they have different values in later implementations.

Vulnerability Details

The getMarketPrices function retrieves prices for indexToken, longToken, and shortToken as follows;

function getMarketPrices(
address market
) internal view returns (MarketPrices memory) {
MarketPrices memory prices;
MarketProps memory marketInfo = gmxReader.getMarket(
address(dataStore),
market
);
address oracle = IOrderHandler(orderHandler).oracle();
prices.indexTokenPrice = IOracle(oracle).getPrimaryPrice(
marketInfo.indexToken
);
prices.longTokenPrice = IOracle(oracle).getPrimaryPrice(
marketInfo.longToken
);
prices.shortTokenPrice = IOracle(oracle).getPrimaryPrice(
marketInfo.shortToken
);
return prices;
}

The prices fetched above are then used in _validatePrice as follows;

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
//...SNIP...
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
_check(marketData.longToken, prices.indexTokenPrice.min);
_check(marketData.longToken, prices.indexTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}

The _check function is used to validate prices, but prices.indexTokenPrice is used for both indexToken and longToken.Currently, since indexToken and longToken are the same, the function behaves correctly.
In the future, if indexToken and longToken become different, using the same price for both will result in incorrect validation and potential mispricing.

Impact

If the assumption breaks, _validatePrice will use the wrong prices for longToken, leading to inaccurate price checks and mispricing.

Tools

Manual Review

Recommendations

Modify _validatePrice to ensure that prices.indexTokenPrice is used only for indexToken and prices.longTokenPrice is used for longToken.

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
//...SNIP...
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
_check(marketData.indexToken, prices.indexTokenPrice.min);
_check(marketData.indexToken, prices.indexTokenPrice.max);
- _check(marketData.longToken, prices.indexTokenPrice.min);
- _check(marketData.longToken, prices.indexTokenPrice.max);
+ _check(marketData.longToken, prices.longTokenPrice.min);
+ _check(marketData.longToken, prices.longTokenPrice.max);
_check(marketData.shortToken, prices.shortTokenPrice.min);
_check(marketData.shortToken, prices.shortTokenPrice.max);
}
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.