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

Incorrect Gas Limit Calculation for Limit, Stop and ADL Orders in GmxProxy lead to Transaction Failure

Summary

The getExecutionGasLimit function in the GmxProxy contract is not handling all order types correctly when calculating gas limits. According to GMX's ExecuteOrderUtils.sol contract, all orders except liquidation and ADL (automatic deleveraging) orders should incur a gas fee based on the swap, increase, and decrease types. However, the current implementation only accounts for a subset of order types, leading to incomplete gas fee calculations and transaction failure.

Vulnerability Details

The existing code in the getExecutionGasLimit function only handles gas limit calculations for MarketIncrease, MarketDecrease, and MarketSwap order types. It fails to consider all limit orders and stop orders:

Limit Orders and Stop Orders: These types of orders should also be charged gas fees, but they are not currently considered.

contracts/GmxProxy.sol:getExecutionGasLimit#L171-L191

function getExecutionGasLimit(
Order.OrderType orderType,
uint256 _callbackGasLimit
) public view returns (uint256 executionGasLimit) {
if (orderType == Order.OrderType.MarketIncrease) {
estimatedGasLimit =
dataStore.getUint(INCREASE_ORDER_GAS_LIMIT) +
gasPerSwap;
} else if (orderType == Order.OrderType.MarketDecrease) {
estimatedGasLimit =
dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
gasPerSwap;
} else if (orderType == Order.OrderType.LimitDecrease) {
estimatedGasLimit =
dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
gasPerSwap;
} else if (orderType == Order.OrderType.StopLossDecrease) {
estimatedGasLimit =
dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
gasPerSwap;
} else if (orderType == Order.OrderType.MarketSwap) {
estimatedGasLimit =
dataStore.getUint(SWAP_ORDER_GAS_LIMIT) +
gasPerSwap;
}
// @audit lack of the estimatedGasLimit for the order type of `StopIncrease` and `LimitIncrease`.

However, all orders except liquidation and ADL (automatic deleveraging) orders should incur a gas fee based on the swap, increase, and decrease types, according to GMX's ExecuteOrderUtils.sol contract.

The incomplete handling of order types leads to erroneous gas calculations for certain orders, impacting both the accuracy of gas estimations, Keeper incentives and transaction failure.

Impact

  1. Incomplete Gas Fee Calculation: Limit orders, stop-loss orders are not charged the correct gas fees, leading to inaccuracies in gas estimations.

  2. This discrepancy may cause Keepers to incorrectly estimate execution costs, leading to inefficient transaction execution or even transaction failures.

POC
contract GmxProxyGasTest {
GmxProxy public gmxProxy;
IExchangeRouter public exchangeRouter;
address public constant WETH = 0x82aF49447D8a07e3bd95BD0d56f35241523fBab1;
// Setup function to initialize necessary contracts and states
function setUp() public {
gmxProxy = new GmxProxy();
// Set necessary contract addresses and initial states
}
// Test 1: Verify gas limits for different order types
function testOrderTypeGasLimits() public {
// 1. Market Order
uint256 marketIncreaseGas = gmxProxy.getExecutionGasLimit(
Order.OrderType.MarketIncrease,
100000
);
// 2. Limit Order
uint256 limitIncreaseGas = gmxProxy.getExecutionGasLimit(
Order.OrderType.LimitIncrease,
100000
);
// 3. Liquidation Order
uint256 liquidationGas = gmxProxy.getExecutionGasLimit(
Order.OrderType.Liquidation,
100000
);
console.log("Market Increase Gas:", marketIncreaseGas);
console.log("Limit Increase Gas:", limitIncreaseGas);
console.log("Liquidation Gas:", liquidationGas);
// Verify that gas limits meet expectations
assert(marketIncreaseGas > 0);
assert(limitIncreaseGas > 0);
assert(liquidationGas == 0);
}
// Test 2: Verify actual order creation and execution
function testOrderExecution() public {
// 1. Create Market Order
IGmxProxy.OrderData memory marketOrderData = IGmxProxy.OrderData({
market: WETH,
indexToken: WETH,
initialCollateralToken: WETH,
swapPath: new address isLong: true,
sizeDeltaUsd: 1000e18,
initialCollateralDeltaAmount: 1e18,
amountIn: 1e18,
callbackGasLimit: 0,
acceptablePrice: 1800e8,
minOutputAmount: 0
});
bytes32 marketKey = gmxProxy.createOrder(
Order.OrderType.MarketIncrease,
marketOrderData
);
// 2. Create Limit Order
IGmxProxy.OrderData memory limitOrderData = marketOrderData;
bytes32 limitKey = gmxProxy.createOrder(
Order.OrderType.LimitIncrease,
limitOrderData
);
// Retrieve order information and verify gas settings
Order.Props memory marketOrder = exchangeRouter.getOrder(marketKey);
Order.Props memory limitOrder = exchangeRouter.getOrder(limitKey);
console.log("Market Order Gas:", marketOrder.numbers.executionFee);
console.log("Limit Order Gas:", limitOrder.numbers.executionFee);
// Verify that gas fees are set correctly
assert(marketOrder.numbers.executionFee > 0);
assert(limitOrder.numbers.executionFee > 0);
}
// Test 3: Verify special orders (Liquidation/ADL)
function testSpecialOrders() public {
// Simulate a liquidation order
bytes32 liquidationKey = createLiquidationOrder();
Order.Props memory liquidationOrder = exchangeRouter.getOrder(liquidationKey);
console.log("Liquidation Order Gas:", liquidationOrder.numbers.executionFee);
assert(liquidationOrder.numbers.executionFee == 0);
}
}

Tools Used

Manual Code Review

Recommendations

It is recommended to modify the getExecutionGasLimit function to account for all necessary order types, including all limit, stop-loss. Specifically:

  • For Limit and Stop Orders: Add logic to treat them like market orders in terms of gas fee calculation.

  • For Liquidation and ADL Orders: Return a gas fee of 0 for these order types, as they are subsidized by the treasury.

function getExecutionGasLimit(
Order.OrderType orderType,
uint256 gasPerSwap
) public view returns (uint256) {
...
uint256 estimatedGasLimit;
- if (orderType == Order.OrderType.MarketIncrease) {
- estimatedGasLimit =
- dataStore.getUint(INCREASE_ORDER_GAS_LIMIT) +
- gasPerSwap;
- } else if (orderType == Order.OrderType.MarketDecrease) {
- estimatedGasLimit =
- dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
- gasPerSwap;
- } else if (orderType == Order.OrderType.LimitDecrease) {
- estimatedGasLimit =
- dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
- gasPerSwap;
- } else if (orderType == Order.OrderType.StopLossDecrease) {
- estimatedGasLimit =
- dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
- gasPerSwap;
- } else if (orderType == Order.OrderType.MarketSwap) {
- estimatedGasLimit =
- dataStore.getUint(SWAP_ORDER_GAS_LIMIT) +
- gasPerSwap;
- }
+ // Liquidation and ADL orders should have 0 gas limit
+ if (orderType == Order.OrderType.Liquidation || msg.sender == address(adlHandler)) {
+ return 0;
+ }
+ // Handle Increase orders
+ if (orderType == Order.OrderType.MarketIncrease ||
+ orderType == Order.OrderType.LimitIncrease ||
+ orderType == Order.OrderType.StopIncrease) {
+ estimatedGasLimit = dataStore.getUint(INCREASE_ORDER_GAS_LIMIT) +
+ gasPerSwap;
+ }
+ // Handle Decrease orders
+ else if (orderType == Order.OrderType.MarketDecrease ||
+ orderType == Order.OrderType.LimitDecrease ||
+ orderType == Order.OrderType.StopLossDecrease) {
+ estimatedGasLimit = dataStore.getUint(DECREASE_ORDER_GAS_LIMIT) +
+ gasPerSwap;
+ }
+ // Handle Swap orders
+ else if (orderType == Order.OrderType.MarketSwap ||
+ orderType == Order.OrderType.LimitSwap) {
+ estimatedGasLimit = dataStore.getUint(SWAP_ORDER_GAS_LIMIT) +
+ gasPerSwap;
+ }
executionGasLimit =
baseGasLimit +
((estimatedGasLimit + _callbackGasLimit) * multiplierFactor) /
PRECISION;
}
Updates

Lead Judging Commences

n0kto Lead Judge 9 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.

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.

Give us feedback!