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

Array Out-of-Bounds Access in GmxProxy's afterOrderExecution Function

Vulnerability Details

The GmxProxy contract's afterOrderExecution function directly accesses array elements of the eventData parameter without performing bounds checking. Specifically, in the liquidation handling code path, the function attempts to access eventData.uintItems.items[0] and eventData.addressItems.items[0] without verifying if these arrays contain any elements.

function afterOrderExecution(bytes32 requestKey, Order.Props memory order, EventLogData memory eventData) external override validCallback(requestKey, order) {
// ...
if (order.numbers.orderType == Order.OrderType.Liquidation) {
if (eventData.uintItems.items[0].value > 0) { // Vulnerable line
IERC20(eventData.addressItems.items[0].value).safeTransfer(perpVault, eventData.uintItems.items[0].value);
}
// ...
}
// ...
}

This vulnerability is particularly concerning because the function is a critical callback that handles both regular order execution and liquidation events from the GMX protocol. The assumption that the event data arrays will always contain elements is unsafe, as external callbacks should be designed to handle any possible input state.

Proof Of Concept

A coded function in the PerpetualVaultTest.sol

function test_GmxProxy_OutOfBoundsAccess() external {
// First let's do the standard setup for a position that can be liquidated
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
depositFixture(alice, 1e10);
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
// Get the GMX proxy instance
IGmxProxy gmxProxy = PerpetualVault(vault).gmxProxy();
// Create empty Event Log Data
AddressItems memory addressItems;
UintItems memory uintItems;
IntItems memory intItems;
BoolItems memory boolItems;
Bytes32Items memory bytes32Items;
BytesItems memory bytesItems;
StringItems memory stringItems;
EventLogData memory eventData = EventLogData({
addressItems: addressItems,
uintItems: uintItems,
intItems: intItems,
boolItems: boolItems,
bytes32Items: bytes32Items,
bytesItems: bytesItems,
stringItems: stringItems
});
// Create minimal Order Props
Order.Props memory order;
order.numbers.orderType = Order.OrderType.Liquidation;
order.addresses.account = address(gmxProxy);
// Simulate being the OrderHandler
vm.prank(gmxProxy.orderHandler());
vm.expectRevert();
gmxProxy.afterOrderExecution(bytes32(0), order, eventData);
}

Impact

  1. If a liquidation event is triggered with empty event data arrays, the liquidation process will fail due to the array out-of-bounds access, potentially leaving the system in an inconsistent state.

  2. Since the function handles token transfers during liquidation, a revert at this stage could result in funds becoming temporarily or permanently stuck in the contract.

  3. As this function is part of the core GMX integration logic, its failure could disrupt essential protocol operations, particularly during critical market events where liquidations are most likely to occur.

  4. The vulnerability essentially makes the system susceptible to failure when handling certain valid GMX callback data structures, reducing the overall robustness and reliability of the protocol.

Recommendations

  • implement proper array bounds checking before accessing array elements

function afterOrderExecution(bytes32 requestKey, Order.Props memory order, EventLogData memory eventData) external override validCallback(requestKey, order) {
// ...
if (order.numbers.orderType == Order.OrderType.Liquidation) {
// Add bounds checking
if (eventData.uintItems.items.length > 0 &&
eventData.addressItems.items.length > 0 &&
eventData.uintItems.items[0].value > 0) {
IERC20(eventData.addressItems.items[0].value).safeTransfer(perpVault, eventData.uintItems.items[0].value);
}
// ...
}
// ...
}
  • Consider adding input validation functions to verify the structure of eventData before processing.

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.

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.

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.

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.