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

[M-03] Incorrect Position State After Liquidation in `afterLiquidationExecution()`

phaseTwo

Summary

The afterLiquidationExecution() function fails to update positionIsClosed to true after a position is fully liquidated. This results in the vault incorrectly assuming that a position is still active, leading to unexpected contract behavior, including preventing deposits, interfering with subsequent position openings, and causing logical inconsistencies in the vault's operation.

Vulnerability Details

Function Overview

The afterLiquidationExecution() function is triggered after a liquidation or automatic deleveraging (ADL) event. It performs the following key actions:

  1. Ensures that only the GMX proxy can call it.

  2. Pauses deposits after liquidation.

  3. Retrieves the current position size (sizeInTokens).

  4. If sizeInTokens == 0, it deletes the curPositionKey, indicating that the position has been removed.

  5. Updates the flow state depending on whether it was previously in a NONE, DEPOSIT, or WITHDRAW state.

Issue: positionIsClosed is Never Set to true

  • The function correctly deletes curPositionKey but fails to explicitly update positionIsClosed = true.

  • This means the contract still assumes there is an active position when it has already been liquidated.

Code Snippet

function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
​
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) { // If position is fully liquidated
delete curPositionKey;
//@audit-issue: Does not set positionIsClosed = true
}
​
if (flow == FLOW.NONE) {
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
} else if (flow == FLOW.DEPOSIT) {
flowData = sizeInTokens;
} else if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
}
}

Impact

  1. Incorrect Vault State

    • The vault may still believe a position is open after liquidation, leading to incorrect decision-making in other contract functions.

  2. Deposits May Be Blocked Indefinitely

    • Functions that check positionIsClosed before allowing deposits may prevent new deposits even when no position exists.

  3. Withdrawals May Fail or Be Handled Incorrectly

    • Since the contract still thinks a position is open, it may attempt to close a non-existent position, leading to execution failures.

  4. Inconsistent Contract Behavior

    • Functions that depend on accurate position tracking (e.g., compounding, withdrawals, or new position openings) may malfunction or revert unexpectedly.

Tools Used

  • Manual Code Review

Recommendations

1. Explicitly Set positionIsClosed = true After Liquidation

Modify afterLiquidationExecution() to ensure positionIsClosed is properly updated.

Proposed Fix:

function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
​
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
positionIsClosed = true; // Fix: Explicitly mark position as closed
}
​
if (flow == FLOW.NONE) {
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
} else if (flow == FLOW.DEPOSIT) {
flowData = sizeInTokens;
} else if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
}
}

2. Validate positionIsClosed Before Allowing Certain Actions

Ensure other functions check positionIsClosed before allowing deposits, withdrawals, or position openings.

Example Fix:

require(positionIsClosed, "Position must be closed before proceeding");

3. Emit an Event When a Position is Liquidated

To improve off-chain tracking, emit an event to log the position’s closure.

Proposed Event:

event PositionClosed(bytes32 positionKey, uint256 timestamp);

Modify afterLiquidationExecution() to emit the event:

if (sizeInTokens == 0) {
delete curPositionKey;
positionIsClosed = true;
emit PositionClosed(curPositionKey, block.timestamp);
}
Updates

Lead Judging Commences

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

invalid_liquidation_do_not_reset_positionIsClosed

"// keep the positionIsClosed value so that let the keeper be able to create an order again with the liquidated fund" Liquidation can send some remaining tokens. No real impact here.

Support

FAQs

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