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

Permanent ETH Lock in GmxProxy's createOrder Due to Missing Expiry Mechanism

Summary

The createOrder function in GmxProxy.sol lacks an expiry mechanism for GMX orders and doesn't handle execution fee refunds in case of order failures, leading to permanent locking of ETH in the GMX orderVault contract.

Vulnerability Details

When creating an order through createOrder, the function immediately transfers ETH as execution fee to GMX's orderVault without any expiry time or recovery mechanism:

function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
// @audit ETH sent without expiry or recovery mechanism
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);

Key issues:

  1. No expiry timestamp for orders

  2. ETH is sent immediately without waiting for order confirmation

  3. No mechanism to recover ETH if order fails to execute

  4. Relies entirely on GMX's execution which could fail for various reasons

Impact

  1. Permanent loss of ETH if orders fail to execute or get stuck

  2. No way to recover locked ETH in edge cases

  3. Protocol funds could be permanently locked

  4. Accumulated losses over multiple failed orders

Proof of Concept

The POC demonstrates how ETH becomes permanently locked by simulating a scenario where an order is created and ETH is sent as execution fee. Using Foundry's testing framework, we:

  1. Mock the necessary contracts (perpVault and orderVault)

  2. Initialize the proxy with 1 ETH initial balance

  3. Create an order that will require ETH execution fee

  4. Verify that ETH is transferred and locked in orderVault

  5. Show that ETH cannot be recovered even after the order fails

The test proves this is a permanent lock rather than temporary by attempting and failing to recover the funds through the available mechanisms.

This simple POC concretely demonstrates the critical issue: once ETH is sent for an order's execution fee, there is no way to recover it if the order fails or gets stuck.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
import "forge-std/Test.sol";
import "../../contracts/GmxProxy.sol";
import "../../contracts/interfaces/IGmxReader.sol";
contract GmxProxyEthLockTest is Test {
GmxProxy proxy;
address perpVault;
address orderVault;
uint256 initialBalance;
function setUp() public {
// Setup mocked contracts
perpVault = address(new MockPerpVault());
orderVault = address(new MockOrderVault());
proxy = new GmxProxy();
proxy.initialize(
address(0x1), // orderHandler
address(0x2), // liquidationHandler
address(0x3), // adlHandler
address(0x4), // gExchangeRouter
orderVault, // gmxRouter
address(0x6), // dataStore
orderVault, // orderVault
address(0x8), // gmxReader
address(0x9) // referralStorage
);
// Fund proxy with ETH
vm.deal(address(proxy), 1 ether);
initialBalance = address(proxy).balance;
}
function testEthLockOnFailedOrder() public {
// Create order that will fail
vm.startPrank(perpVault);
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
market: address(0x1),
indexToken: address(0x2),
initialCollateralToken: address(0x3),
swapPath: new address[](0),
isLong: true,
sizeDeltaUsd: 1000e30,
initialCollateralDeltaAmount: 0,
amountIn: 100e6,
callbackGasLimit: 1000000,
acceptablePrice: 0,
minOutputAmount: 0
});
// Order creation will send ETH to orderVault
proxy.createOrder(Order.OrderType.MarketIncrease, orderData);
vm.stopPrank();
// Verify ETH is locked in orderVault
assertLt(address(proxy).balance, initialBalance);
assertGt(address(orderVault).balance, 0);
// Attempt to recover ETH - should fail
vm.expectRevert();
proxy.withdrawEth();
}
}
contract MockPerpVault {
// Mock implementation
}
contract MockOrderVault {
// Mock implementation that locks ETH
receive() external payable {}
}

Tools Used

  • Manual code review

  • Foundry testing framework

  • Static analysis

Recommended Mitigation

Add expiry timestamp and refund mechanism:

contract GmxProxy {
uint256 public constant ORDER_EXPIRY = 1 hours;
mapping(bytes32 => uint256) public orderExpiries;
function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
require(msg.sender == perpVault, "invalid caller");
uint256 positionExecutionFee = getExecutionGasLimit(
orderType,
orderData.callbackGasLimit
) * tx.gasprice;
require(
address(this).balance >= positionExecutionFee,
"insufficient eth balance"
);
// Set expiry for the order
bytes32 orderId = _generateOrderId(orderType, orderData);
orderExpiries[orderId] = block.timestamp + ORDER_EXPIRY;
gExchangeRouter.sendWnt{value: positionExecutionFee}(
orderVault,
positionExecutionFee
);
return orderId;
}
function recoverExpiredOrderEth(bytes32 orderId) external {
require(msg.sender == perpVault, "invalid caller");
require(
block.timestamp > orderExpiries[orderId],
"order not expired"
);
// Recover ETH through GMX's cancellation mechanism
gExchangeRouter.cancelOrder(orderId);
delete orderExpiries[orderId];
}
}

Key changes:

  1. Added order expiry tracking

  2. Implemented recovery mechanism for expired orders

  3. Added validation on expiry time

  4. Integrated with GMX's cancellation system

This ensures ETH can be recovered if orders fail to execute within the expiry window.

Updates

Lead Judging Commences

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

invalid_no_refund_during_cancellation

Order is not executed, those fees can be used for the next retry.

Support

FAQs

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