Liquidity Management

Root Cause & issue LOC.

Within the _cancelFlow() function, when flow == FLOW.WITHDRAW, the contract attempts to refund execution fees by referencing depositInfo[counter] instead of using the deposit ID stored in flowData. However, for a withdrawal flow, the active deposit ID to be refunded is flowData, not counter.

Excerpt from _cancelFlow():

In contrast, when a user calls withdraw(...), the relevant deposit ID is stored in:

Hence, under FLOW.WITHDRAW, the deposit ID of interest is flowData, not counter. Using counter here will refund the wrong deposit’s fee or potentially revert if no deposit info is stored at depositInfo[counter].

Attack / Impact Scenario

1. Setup:

   • Let’s say user A has an active withdrawal in progress for depositId = 5.

   • Meanwhile, counter might be 6 or 7 after other users deposited.

2. Keeper (or any authorized caller) calls cancelFlow() while flow == FLOW.WITHDRAW.

   • The code incorrectly executes:

     refundExecutionFee(

     depositInfo[counter].owner,

     depositInfo[counter].executionFee

     );

     ​

   • This refunds the execution fee to the owner of depositInfo[counter] instead of the owner of depositInfo[flowData].

   • If depositInfo[counter] is a different deposit or does not even exist, the outcome ranges from:

     • Sending the fee to the wrong user, effectively stealing fee refunds from the rightful owner of deposit #5.

     • Or reverting the call if depositInfo[counter] has never been set or is invalid, getting the protocol stuck.

3. Impact:

   User A (with deposit #5) never receives their rightful refund of execution fees.

   Another deposit’s owner might get those fees undeservedly, or the transaction reverts.

Because _cancelFlow() is restricted by gmxLock and onlyKeeper, the practical exploit could come from:

• A malicious keeper, who can systematically siphon or misdirect refunds.

• An incorrect operational call by the legitimate keeper, causing refunds to go to the wrong deposit.

Either way, the bug is real and reproducible whenever cancelFlow() is called during a withdrawal flow.

Severity

This is a medium severity issue. It does not directly let an attacker drain the entire vault but can cause a misdirection or loss of user funds (the execution fee refunds). It also breaks the invariant that each withdrawing user should recover any leftover fee they posted.

FIX

In _cancelFlow(), when flow == FLOW.WITHDRAW, use flowData instead of counter: