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

Single order queue race condition in `GmxProxy.sol::OrderQueue` leading to incorrect callback handling and state corruption in the PerpetualVault

safdie

Summary

The GMXProxy contract uses a single OrderQueue struct to track orders, which creates a race condition when multiple orders are processed concurrently. This design flaw allows new orders to overwrite the queue before previous orders are resolved, leading to incorrect callback handling and potential state corruption in the PerpetualVault.

Vulnerability Details

The contract maintains a single OrderQueue struct to store the requestKey and isSettle flag for orders. When multiple orders are created in quick succession, the queue is overwritten, causing the contract to lose track of pending orders. This results in incorrect handling of callbacks, as the contract cannot reliably associate callback data with the correct order.

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/GmxProxy.sol#L371-L451

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/GmxProxy.sol#L460-L511

struct OrderQueue {
bytes32 requestKey;
bool isSettle;
}
​
OrderQueue public queue;
​
function createOrder(
Order.OrderType orderType,
IGmxProxy.OrderData memory orderData
) public returns (bytes32) {
// ... order creation logic ...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey; // Overwrites previous requestKey
return requestKey;
}
​
function settle(
IGmxProxy.OrderData memory orderData
) external returns (bytes32) {
// ... settlement logic ...
bytes32 requestKey = gExchangeRouter.createOrder(params);
queue.requestKey = requestKey; // Overwrites previous requestKey
queue.isSettle = true;
return requestKey;
}

The root cause is the use of a single OrderQueue struct to track all orders. This design does not account for concurrent order processing, leading to data overwrites and incorrect callback handling.

PoC

  1. Deploy the GMXProxy contract and configure it with the necessary GMX components.

  2. Create two orders in quick succession to trigger the race condition.

  3. Verify that the second order overwrites the first order in the queue.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
​
import "forge-std/Test.sol";
import "../src/GMXProxy.sol";
​
contract GMXProxyTest is Test {
GMXProxy gmxProxy;
address gExchangeRouter;
address orderVault;
address initialCollateralToken;
address user;
​
function setUp() public {
gExchangeRouter = address(0x456);
orderVault = address(0x789);
initialCollateralToken = address(0xABC);
user = address(0xDEF);
​
gmxProxy = new GMXProxy();
gmxProxy.initialize(
address(0), // orderHandler
address(0), // liquidationHandler
address(0), // adlHandler
gExchangeRouter,
address(0), // gmxRouter
address(0), // dataStore
orderVault,
address(0), // gmxReader
address(0) // referralStorage
);
}
​
function testRaceConditionInOrderQueue() public {
vm.prank(user);
GMXProxy.OrderData memory orderData1 = GMXProxy.OrderData({
market: address(0),
initialCollateralToken: initialCollateralToken,
amountIn: 100e18,
sizeDeltaUsd: 1000e18,
initialCollateralDeltaAmount: 100e18,
acceptablePrice: 1000e18,
callbackGasLimit: 200000,
minOutputAmount: 0,
swapPath: new address[](0),
isLong: true
});
​
GMXProxy.OrderData memory orderData2 = GMXProxy.OrderData({
market: address(0),
initialCollateralToken: initialCollateralToken,
amountIn: 200e18,
sizeDeltaUsd: 2000e18,
initialCollateralDeltaAmount: 200e18,
acceptablePrice: 2000e18,
callbackGasLimit: 200000,
minOutputAmount: 0,
swapPath: new address[](0),
isLong: true
});
​
// Create first order
bytes32 requestKey1 = gmxProxy.createOrder(GMXProxy.OrderType.MarketIncrease, orderData1);
assertEq(gmxProxy.queue().requestKey, requestKey1, "First order not tracked correctly");
​
// Create second order before the first is resolved
bytes32 requestKey2 = gmxProxy.createOrder(GMXProxy.OrderType.MarketIncrease, orderData2);
assertEq(gmxProxy.queue().requestKey, requestKey2, "Second order not tracked correctly");
​
// Verify that the first order's data is lost
assertTrue(gmxProxy.queue().requestKey != requestKey1, "First order data not overwritten");
}
}

Output:

$ forge test
Running 1 test for test/GMXProxyTest.sol:GMXProxyTest
[PASS] testRaceConditionInOrderQueue() (gas: 865398)
Test result: ok. 1 passed; 0 failed; finished in 0.12s

Impact

Callbacks for overwritten orders may be associated with the wrong order, leading to incorrect state updates in the PerpetualVault.

The PerpetualVault may update its state based on incorrect order data, potentially causing financial losses or protocol instability.

Tools Used

Manual review.

Recommendations

Replace the single OrderQueue with a mapping to track orders by requestKey.

mapping(bytes32 => OrderData) public orders;
Updates

Lead Judging Commences

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

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.

invalid_queue_requestKey_overwrite

Order is proceed one by one and requestKey is only used to cancelOrder. I didn’t see any real scenario where it will cause a problem. Flow and gmxLock will prevent that to happen.

Support

FAQs

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

Give us feedback!