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

Restarting withdraw flow in `afterLiquidationExecution` may cause double withdraw and mess up accounting.

Summary

Restarting withdraw flow in afterLiquidationExecution may cause double withdraw and mess up accounting.

Vulnerability Details

First, let's recall the process for a user withdraw (position is open and maintains an Gmx position).

  1. User calls withdraw(). Setting nextAction.selector=WITHDRAW_ACTION.

  2. Keeper calls runNextAction() and creates a decreasePosition order.

  3. Gmx callback afterOrderExecution(), setting nextAction.selector=FINALIZE.

  4. Keeper calls runNextAction() and finalizes the flow.

Gmx has an liquidation/ADL handler, which is used for risk handling. It will trigger the afterLiquidationExecution() function, and when it is triggered during the withdraw flow, it will set nextAction.selector=WITHDRAW_ACTION to restart the entire workflow.

The issue is, if afterLiquidationExecution() is triggered after step 2 and before step 4, the entire withdraw flow is forced to start all over again, but the position in Gmx was already decreased. There are multiple impacts to this:

  1. After liquidation/adl happens, asset/share ratio decreases. This leads to users receiving less assets then he should, since he triggered the withdraw before liquidation/adl.

  2. The second withdraw may fail completely due to not enough funds in position (e.g. user withdraws >50% of the original position size).

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/PerpetualVault.sol#L581

function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
}
if (flow == FLOW.NONE) {
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
} else if (flow == FLOW.DEPOSIT) {
flowData = sizeInTokens;
} else if (flow == FLOW.WITHDRAW) {
// restart the withdraw flow even though current step is FINALIZE.
@> nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
}
}

Impact

See above.

Tools Used

N/A

Recommendations

Don't restart the workflow, just let the user get what he withdrawed for.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
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.