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

Missing GMX Lock Protection on Position Management Function

tiannah

Description

The run() function in the PerpetualVault contract lacks the gmxLock modifier, while its companion function runNextAction() implements it. This inconsistency allows the run() function to be called during ongoing GMX operations, potentially corrupting position states and breaking core vault invariants.

The vulnerability stems from the fact that position management operations with GMX are asynchronous, requiring a callback. During the period between initiating a GMX operation and receiving its callback, the run() function can be invoked again due to missing the protective gmxLock modifier.

The two functions:

// Missing gmxLock modifier
function run(
bool isOpen,
bool isLong,
MarketPrices memory prices,
bytes[] memory metadata
) external nonReentrant { // <- vulnerable here
// ... position management logic
}
// The properly protected function
function runNextAction(MarketPrices memory prices, bytes[] memory metadata)
external nonReentrant gmxLock { // <- has protection
// ... next action logic
}

Tool Used

  • Manual code review

  • Foundry testing framework

Impact

The vulnerability can lead to:

  1. Position state corruption

  2. Violation of vault leverage consistency

  3. Incorrect next action execution

  4. Potential loss of funds due to mismanaged positions

  5. Breaking of core protocol invariants, specifically:

    • "After all actions are completed, nextAction should be empty"

    • "The leverage of each vault will stay consistent from start to finish"

Proof of Concept

function testVulnerability() public {
// Set up test environment
address user = makeAddr("user");
address keeper = makeAddr("keeper");
vm.label(user, "User");
vm.label(keeper, "Keeper");
// Deposit initial funds
vm.deal(user, 1 ether);
vm.prank(user);
vault.deposit(1 ether);
// Set up mock prices
MarketPrices memory prices = MarketPrices({
shortTokenPrice: PriceProps({ min: 1e30, max: 1e30 }),
indexTokenPrice: PriceProps({ min: 1e30, max: 1e30 })
});
// First position change (opens long position)
vm.prank(keeper);
bytes[] memory metadata = new bytes[](1);
vault.run(true, true, prices, metadata);
// Check that position is being opened
assertEq(vault.isLock(), true);
// Attempt second position change while first is still in progress
vm.expectRevert(bytes("GMX lock"));
vault.run(false, false, prices, metadata);
// Complete first position
vm.prank(keeper);
bytes32 requestKey = bytes32("test-request");
vault.afterOrderExecution(requestKey, bytes32("test-position"),
IGmxProxy.OrderResultData({
orderType: IGmxProxy.OrderType.MarketIncrease,
isSettle: false,
sizeDeltaUsd: 1e30,
outputAmount: 1e30
}), prices);
// Verify position state
assertEq(vault.isLock(), false);
assertEq(vault.positionIsClosed(), false);
assertEq(vault.beenLong(), true);
}

This test demonstrates the vulnerability by:

  1. Setting up a test environment with a user and keeper

  2. Making an initial deposit

  3. Attempting to open a long position

  4. Verifying the GMX lock is active

  5. Attempting a second position change while the first is in progress

  6. Completing the first position

  7. Verifying the final position state

The test proves that the gmxLock modifier prevents concurrent position changes, which is crucial for maintaining position state integrity and preventing potential financial losses.

Mitigation

Add the gmxLock modifier to the run() function:

function run(
bool isOpen,
bool isLong,
MarketPrices memory prices,
bytes[] memory metadata
) external nonReentrant gmxLock { // Added gmxLock
// ... existing logic
}

This ensures position management operations cannot overlap with ongoing GMX operations, maintaining state consistency and protocol invariants.

Conclusion

The missing gmxLock modifier on the run() function represents a critical vulnerability in the position management system. While runNextAction() is properly protected, the unprotected run() function creates a window where position states can be corrupted during GMX operations. This inconsistency in protection mechanisms could lead to significant issues with position management and potential financial losses. The fix is straightforward - adding the gmxLock modifier to match the protection level of runNextAction().

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.