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

Execution fee calculation does not provide real buffer which will revert order executions

Summary

Current implementation for calculating execution fee for GMX orders does not provide buffer which will make orders revert when gas price when executing order is higher than when order is created.

Vulnerability Details

In GMX increasing/decreasing position is two-step process - first an order is created and then order is executed by GMX keepers. In both creation and execution, the supplied execution fee is verified to be above certain threshold calculated as follows:

function validateExecutionFee(DataStore dataStore, uint256 estimatedGasLimit, uint256 executionFee, uint256 oraclePriceCount) internal view {
uint256 gasLimit = adjustGasLimitForEstimate(dataStore, estimatedGasLimit, oraclePriceCount);
uint256 minExecutionFee = gasLimit * tx.gasprice;
if (executionFee < minExecutionFee) {
revert Errors.InsufficientExecutionFee(minExecutionFee, executionFee);
}
}
function adjustGasLimitForEstimate(DataStore dataStore, uint256 estimatedGasLimit, uint256 oraclePriceCount) internal view returns (uint256) {
uint256 baseGasLimit = dataStore.getUint(Keys.ESTIMATED_GAS_FEE_BASE_AMOUNT_V2_1);
baseGasLimit += dataStore.getUint(Keys.ESTIMATED_GAS_FEE_PER_ORACLE_PRICE) * oraclePriceCount;
uint256 multiplierFactor = dataStore.getUint(Keys.ESTIMATED_GAS_FEE_MULTIPLIER_FACTOR);
uint256 gasLimit = baseGasLimit + Precision.applyFactor(estimatedGasLimit, multiplierFactor);
return gasLimit;
}

Where estimatedGasLimit is calculated as follows:

function estimateExecuteSwapOrderGasLimit(DataStore dataStore, Order.Props memory order) internal view returns (uint256) {
uint256 gasPerSwap = dataStore.getUint(Keys.singleSwapGasLimitKey());
return dataStore.getUint(Keys.swapOrderGasLimitKey()) + gasPerSwap * order.swapPath().length + order.callbackGasLimit();
}

executeOrder() check: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/exchange/OrderHandler.sol#L210
createOrder() check: https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/order/OrderUtils.sol#L154-L155

In Gamma we have the same logic for calculating the executionFee: https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/GmxProxy.sol#L155C1-L197C4

In the end of getExecutionGasLimit we have the following snippet:

// multiply 1.2 (add some buffer) to ensure that the creation transaction does not revert.
executionGasLimit =
baseGasLimit +
((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) /
PRECISION;

The multiplication with multiplierFactor is not the buffer described in GMX docs (https://docs.gmx.io/docs/api/contracts-v2#execution-fee). It is part of the threshold calculation which is used to verify sufficient fee is supplied.

Position execution fee is calculated as follows in GmxProxy:

uint256 positionExecutionFee = getExecutionGasLimit(
Order.OrderType.MarketDecrease,
orderData.callbackGasLimit
) * tx.gasprice;

We can have the following scenario:
Calculated callbackGasLimit is 1_000_000 with tx.gasprice equal to 5 (values are demonstrative).
This makes supplied execution fee = 5_000_000.
createOrder() in GMX will be executed successfully as passed execution fee is equal to the minimum amount needed (same tx = same gas price).
Few blocks later keeper will execute the transaction but if the gas price is > 5, lets say 6, the minimum threshold for execution fee in GMX would be 6_000_000. Now the transaction will be cancelled.

Impact

Temporal DoS for deposit/withdrawal. Due to time-sensitiveness of protocol and moving prices, users may lose funds because of inability to withdraw or lose potential value because of inability to deposit.

Tools Used

Manual review.

Recommendations

Consider implementing a buffer as described in the GMX docs.

Updates

Lead Judging Commences

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

invalid_tx-gasprice_instable

The frontrunner won’t trigger "congestion" without a huge amount of transactions, and it will cost a lot. Moreover, the execution gas limit is overestimated to prevent such cases: ``` executionGasLimit = baseGasLimit + ((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) / PRECISION; ``` The keeper won’t wait long to execute the order; otherwise, GMX would not be competitive.

Support

FAQs

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