Liquidity Management

Summary

The _handleReturn function facilitates the processing of collateral returns during withdrawals, manages the burning of deposit records, and refunds any excess execution fees. However, the refund logic mistakenly references a global counter instead of the specific depositId associated with the withdrawal. Additionally, this refund process occurs after the deposit record has been deleted. These flaws may result in refunds being applied incorrectly or not at all, leading to potential loss of depositor value and violating the principle that a depositor’s share value must remain intact.

Vulnerability Details

• State Deletion Before Refund:
  The function invokes _burn(depositId) to remove the deposit record before executing the refund of excess execution fees. This deletion means critical data, such as executionFee and owner, may no longer be accessible when needed.

• Incorrect Use of Global Counter:
  Instead of referencing depositInfo[depositId], the refund logic incorrectly relies on depositInfo[counter]:

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

  ​

  Here, counter is a global variable that increments with each new deposit. If additional deposits occur between the initiation and completion of a withdrawal, counter may point to an unrelated deposit record, leading to incorrect refund calculations.

• Potential Mismatch Consequences:
  Due to this flaw, refunds might be miscalculated or sent to an unintended recipient, affecting the accuracy of share value computations and disrupting the fair distribution of funds.

  Furthermore, even if counter happens to match depositId, the deposit record would have already been deleted from depositInfo, effectively resetting its data and making it inaccessible.

Impact

• Depositor Fund Loss:
  If the refund is not processed correctly, users may not receive their full excess execution fee, effectively reducing their withdrawal amount and causing unintended financial losses.

• Violation of Protocol Invariants:
  The protocol guarantees that a depositor’s share value remains unaffected by the actions of others. However, this flaw breaks that guarantee by allowing one user’s withdrawal process to disrupt the accuracy of remaining share valuations.

Tools Used

• Manual Code Review

Recommendations

1. Use the Correct Deposit ID:
   Update the refund logic to correctly reference depositInfo[depositId] instead of depositInfo[counter], ensuring that the refund is processed based on the correct deposit record:

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

   ​

2. Reorder Operations:
   Store the necessary executionFee and owner data in local variables before calling _burn(depositId). This ensures the refund logic retains access to the required information even after the deposit record has been deleted.