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

Callback Reentrancy Vulnerability in GMXProxy Contract

Summary

A critical reentrancy vulnerability has been identified in the GMXProxy contract that could allow attackers to manipulate fund flows and drain assets during order execution. The vulnerability occurs due to improper ordering of state updates and external calls in the afterOrderExecution function, allowing malicious contracts to re-enter and manipulate the contract's state before transaction finalization.

Vulnerability Details

Root Cause

The vulnerability exists in the afterOrderExecution function where external calls are made before state updates:

function afterOrderExecution(
bytes32 requestKey,
Order.Props memory order,
EventLogData memory eventData
) external override validCallback(requestKey, order) {
bytes32 positionKey = keccak256(
abi.encode(
address(this),
order.addresses.market,
order.addresses.initialCollateralToken,
order.flags.isLong
)
);
MarketPrices memory prices = getMarketPrices(order.addresses.market);
// Claim funding fees for non-swap orders
if (order.numbers.orderType != Order.OrderType.MarketSwap) {
address[] memory markets = new address[](2);
address[] memory tokens = new address[](2);
markets[0] = order.addresses.market;
markets[1] = order.addresses.market;
MarketProps memory marketInfo = gmxReader.getMarket(
address(dataStore),
order.addresses.market
);
tokens[0] = marketInfo.indexToken;
tokens[1] = marketInfo.shortToken;
try
gExchangeRouter.claimFundingFees(markets, tokens, perpVault)
returns (uint256[] memory claimedAmounts) {
emit ClaimPositiveFundingFees(tokens[0], claimedAmounts[0], tokens[1], claimedAmounts[1]);
} catch {
emit ClaimPositiveFundingFeeExecutionError(markets, tokens, perpVault);
}
}
if (order.numbers.orderType == Order.OrderType.Liquidation) {
if (eventData.uintItems.items[0].value > 0) {
IERC20(eventData.addressItems.items[0].value).safeTransfer(perpVault, eventData.uintItems.items[0].value);
}
if (eventData.uintItems.items[1].value > 0) {
IERC20(eventData.addressItems.items[1].value).safeTransfer(perpVault, eventData.uintItems.items[1].value);
}
IPerpetualVault(perpVault).afterLiquidationExecution();
} else if (msg.sender == address(adlHandler)) {
uint256 sizeInUsd = dataStore.getUint(keccak256(abi.encode(positionKey, SIZE_IN_USD)));
if (eventData.uintItems.items[0].value > 0) {
IERC20(eventData.addressItems.items[0].value).safeTransfer(perpVault, eventData.uintItems.items[0].value);
}
if (eventData.uintItems.items[1].value > 0) {
IERC20(eventData.addressItems.items[1].value).safeTransfer(perpVault, eventData.uintItems.items[1].value);
}
if (sizeInUsd == 0) {
IPerpetualVault(perpVault).afterLiquidationExecution();
}
} else {
// Determine output token and amount for swap or decrease orders
address outputToken;
uint256 outputAmount;
if (
order.numbers.orderType == Order.OrderType.MarketSwap ||
order.numbers.orderType == Order.OrderType.MarketDecrease
) {
outputToken = eventData.addressItems.items[0].value;
outputAmount = eventData.uintItems.items[0].value;
}
// Construct order result data and notify perpetual vault
IGmxProxy.OrderResultData memory orderResultData = IGmxProxy.OrderResultData(
order.numbers.orderType,
order.flags.isLong,
order.numbers.sizeDeltaUsd,
outputToken,
outputAmount,
queue.isSettle
);
IPerpetualVault(perpVault).afterOrderExecution(requestKey, positionKey, orderResultData, prices);
delete queue;
}
}

Impact

The vulnerability could allow attackers to:

  • Manipulate fund flows during order execution

  • Double-spend or drain funds

  • Compromise the integrity of the GMX protocol integration

Tools Used

  • Foundry

  • Forge for test execution

Mitigation

To fix this vulnerability, implement the following changes:

  1. State Updates First Pattern:

function afterOrderExecution(
bytes32 requestKey,
Order.Props memory order,
EventLogData memory eventData
) external override validCallback(requestKey, order) {
// Update state first
delete queue;
// Then make external calls
if (order.numbers.orderType != Order.OrderType.MarketSwap) {
try gExchangeRouter.claimFundingFees(markets, tokens, perpVault) {
emit ClaimPositiveFundingFees(tokens[0], claimedAmounts[0], tokens[1], claimedAmounts[1]);
} catch {
emit ClaimPositiveFundingFeeExecutionError(markets, tokens, perpVault);
}
}
}
  1. Reentrancy Guard:

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract GmxProxy is IGmxProxy, IOrderCallbackReceiver, Initializable, Ownable2StepUpgradeable, ReentrancyGuard {
// ... existing code ...
function afterOrderExecution(
bytes32 requestKey,
Order.Props memory order,
EventLogData memory eventData
) external override validCallback(requestKey, order) nonReentrant {
// ... function implementation ...
}
}
  1. Callback Validation:

modifier validCallback(bytes32 key, Order.Props memory order) {
require(
msg.sender == address(orderHandler) ||
msg.sender == address(liquidationHandler) ||
msg.sender == address(adlHandler),
"invalid caller"
);
require(order.addresses.account == address(this), "not mine");
require(!isReentrancyInProgress, "reentrancy detected");
_;
}
Updates

Lead Judging Commences

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

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

Support

FAQs

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