Liquidity Management

Summary

The _handleReturn function is responsible for processing collateral returns during withdrawals, burning the corresponding deposit record, and refunding any excess execution fees. However, the refund fee logic incorrectly references the global counter instead of the specific depositId for the withdrawal. Additionally, this refund logic is executed after the deposit record is burned. These issues can lead to refunds being applied to the wrong deposit record or not at all, potentially causing depositor value loss and violating the invariant that a depositor’s share value must be preserved.

Vulnerability Details

• State Deletion Before Refund:
  The function calls _burn(depositId) to remove the deposit record before attempting to refund the excess execution fee. As a result, the necessary data (such as executionFee and owner) is no longer reliably accessible.

• Global Counter Misuse:
  The refund logic uses depositInfo[counter]:

  try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}

  instead of referencing depositInfo[depositId], where depositId is extracted from flowData and represents the specific deposit being processed. Since the global counter increments with each new deposit, if any deposits occur between the withdrawal initiation and its completion, the global counter may point to a different deposit record.

• Resulting Mismatch:
  These issues may cause the refund to be calculated based on incorrect data or be sent to the wrong owner, undermining the accuracy of share value calculations and the fairness of fund distribution.

  And even if the counter and the depositId are same, its already deleted from the depositInfo and so resetted.

Impact

• Financial Loss to Depositors:
  Depositors may not receive the full excess execution fee refund, effectively reducing their withdrawal amount and causing a loss in their deposited value.

• Violation of Invariants:
  The protocol’s invariant—ensuring that a depositor's share value is preserved regardless of other users’ actions—is broken, as one depositor’s withdrawal can inadvertently reduce the value of remaining shares.

Tools Used

• Manual Code Review:

Recommendations

1. Reference the Correct Deposit ID:
   Modify the refund logic to use depositInfo[depositId] (where depositId is obtained from flowData) instead of depositInfo[counter]:

   try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[depositId].owner, depositInfo[depositId].executionFee - usedFee) {} catch {}

   This ensures that the refund is based on the correct deposit record.

2. Reorder Operations:
   Capture the necessary execution fee and owner information in local variables before calling _burn(depositId). This way, even after the deposit record is deleted, the refund logic still has access to the required data.