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

Price Validation Allows Min Price Greater Than Max Price Leading to Incorrect Position Calculations

codertjay

Summary

The KeeperProxy's _validatePrice function does not validate that minimum prices are less than maximum prices in
MarketPrices. When these prices are passed to PerpetualVault's critical functions through the run() function, it can
lead to incorrect position calculations and value accounting.
There are some places in the perpetualVault like the _totalAmount function that could be expecting the min the be less and uses it in calculations
to get total.

I also know we have priceDiffThreshold for all tokens. But the check is against chainlink which is helpful for price difference but in a traditional system min is supposed to be less than max and if its not then its supposed to have been stated on the docs because its used in many calculations on the Perpetualvault.sol contract

Vulnerability Details

In KeeperProxy:

In this case the min can be greater than the max price.

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// ...
_check(marketData.indexToken, prices.indexTokenPrice.min); // min price check
_check(marketData.indexToken, prices.indexTokenPrice.max); // max price check separately
// No validation that min <= max
}

This allows invalid price scenarios like:

prices.indexTokenPrice = PriceProps({
min: 3386184003773116, // Higher than max
max: 3386164003773119 // Lower than min
});

These unvalidated prices are then used in PerpetualVault for critical calculations:

  1. Position Size Calculation:

function _createIncreasePosition(bool _isLong, uint256 acceptablePrice, MarketPrices memory prices) internal {
// Incorrect max price could lead to wrong position size
uint256 sizeDelta = prices.shortTokenPrice.max * amountIn * leverage / BASIS_POINTS_DIVISOR;
}

  1. Vault Value Calculation:

function _totalAmount(MarketPrices memory prices) internal view returns (uint256) {
// Uses min price which could be larger than max
uint256 total = IERC20(indexToken).balanceOf(address(this))
* prices.indexTokenPrice.min
/ prices.shortTokenPrice.min;
return total;
}

and if there is any scenario they where expecting the min to be greater than max this could completely affect the calculations which it is already affecting the totalAmount in here

  1. Its also been used to calculate feeAmount on afterOrderExecution on line 25 below

function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
// @audit: only gmxProxy() can call this function
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
// MarketPrices memory marketPrices = gmxProxy.getMarketPrices(market);
​
// @audit: this set gmxLock to false but if it was true what could happen
_gmxLock = false;
// If the current action is `settle`
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
emit GmxPositionCallbackCalled(requestKey, true);
return;
}
if (orderResultData.orderType == Order.OrderType.MarketIncrease) {
curPositionKey = positionKey;
if (flow == FLOW.DEPOSIT) {
uint256 amount = depositInfo[counter].amount;
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false)
/ prices.shortTokenPrice.min;// its used in here
uint256 prevSizeInTokens = flowData;
// .... rest of code

Proof of Concept

The test can be added on KeeperProxy.t.sol test

​
​
​
function test_MinPriceCanBeGreaterThanMaxPrice() public {
​
address gmxProxyAddress = address(PerpetualVault(vault).gmxProxy());
GmxProxy gmxProxy = GmxProxy(payable(gmxProxyAddress));
​
// Setup: Create a deposit first
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
​
// Get USDC token
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
​
// Instead of using deal, we can either:
address whale = 0x489ee077994B6658eAfA855C308275EAd8097C4A; // Arbitrum USDC whale
vm.startPrank(whale);
uint256 amount = 1e10;
collateralToken.transfer(alice, amount);
vm.stopPrank();
​
// Now proceed with deposit
vm.startPrank(alice);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, amount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
​
​
​
// Set the market price which is not validated ( making min price more than max price )
MarketPrices memory prices = mockData.getMarketPrices();
prices.indexTokenPrice = PriceProps({min: 3386184003773119, max: 3386184003773116}); // min ends with 9 here and max end with 6
prices.longTokenPrice = PriceProps({min: 3386184003773119, max: 3386184003773116}); // min ends with 9 here and max end with 6
prices.shortTokenPrice = PriceProps({min: 1000029671490240875000009, max: 1000029671490240875000000}); // min ends with 9 here and max end with 0
bytes[] memory data = new bytes[](2);
data[0] = abi.encode(3380000000000000);
​
// Get owner of keeper to execute order
address keeperOwner = KeeperProxy(keeper).owner();
vm.startPrank(keeperOwner);
KeeperProxy(keeper).run(vault, true, false, prices, data);
vm.stopPrank();
​
​
// Verify that min price is higher the max price
assertGt(prices.indexTokenPrice.min , prices.indexTokenPrice.max, "Code is Perfect min Price is less than max");
assertGt(prices.longTokenPrice.min , prices.longTokenPrice.max, "Code is Perfect min Price is less than max");
assertGt(prices.shortTokenPrice.min , prices.shortTokenPrice.max, "Code is Perfect min Price is less than max");
}

The Foundry Shell

forge test --mt test_MinPriceCanBeGreaterThanMaxPrice -vv --via-ir --rpc-url arbitrum
This test check if the min price is greater than the mac price that was used and also there wasn't any revert handling the test

➜ 2025-02-gamma git:(main) ✗ forge test --mt test_MinPriceCanBeGreaterThanMaxPrice -vv --via-ir --rpc-url arbitrum
[⠊] Compiling...
[⠘] Compiling 1 files with Solc 0.8.28
[⠃] Solc 0.8.28 finished in 13.13s
Compiler run successful with warnings:
​
Ran 1 test for test/KeeperProxy.t.sol:KeeperProxyTest
[PASS] test_MinPriceCanBeGreaterThanMaxPrice() (gas: 1639024)
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 16.87ms (10.28ms CPU time)
​
Ran 1 test suite in 1.15s (16.87ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)
➜ 2025-02-gamma git:(main) ✗
​

Impact

HIGH severity because:

  1. Affects Core Position Calculations:

  • Size calculations use shortTokenPrice.max

  • Total value calculations use indexTokenPrice.min

  • When min > max, these calculations become unreliable

  1. Share Value Impact:

function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
// Share calculation depends on _totalAmount() which uses min prices
totalAmountBefore = _totalAmount(prices) - amount;
_shares = amount * totalShares / totalAmountBefore;
}

  1. Affects Multiple Operations:

  • Position increases/decreases

  • Value calculations

  • Share minting/burning

Tools Used

  • Manual code review

Recommendations

Add min/max price relationship validation:
This way the min price will always be less than the max price or equal to it.

function _validatePrice(address perpVault, MarketPrices memory prices) internal view {
// Add price range validation
require(prices.indexTokenPrice.min <= prices.indexTokenPrice.max,"Invalid index price range");
require(prices.longTokenPrice.min <= prices.longTokenPrice.max,"Invalid short price range");
require(prices.shortTokenPrice.min <= prices.shortTokenPrice.max,"Invalid short price range");
​
// Rest of validation...
address market = IPerpetualVault(perpVault).market();
IVaultReader reader = IPerpetualVault(perpVault).vaultReader();
MarketProps memory marketData = reader.getMarket(market);
​
_check(marketData.indexToken, prices.indexTokenPrice.min);
/// Rest of code .....
}
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.

Admin is trusted / Malicious keepers

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. Keepers are added by the admin, there is no "malicious keeper" and if there is a problem in those keepers, that's out of scope. ReadMe and known issues states: " * System relies heavily on keeper for executing trades * Single keeper point of failure if not properly distributed * Malicious keeper could potentially front-run or delay transactions * Assume that Keeper will always have enough gas to execute transactions. There is a pay execution fee function, but the assumption should be that there's more than enough gas to cover transaction failures, retries, etc * There are two spot swap functionalies: (1) using GMX swap and (2) using Paraswap. We can assume that any swap failure will be retried until success. " " * Heavy dependency on GMX protocol functioning correctly * Owner can update GMX-related addresses * Changes in GMX protocol could impact system operations * We can assume that the GMX keeper won't misbehave, delay, or go offline. " "Issues related to GMX Keepers being DOS'd or losing functionality would be considered invalid."

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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