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

Incorrect deposit ID usage in execution fee refund logic leads to wrong refunds

incogknito

Summary

In the PerpetualVault contract's withdrawal process, there is a bug in the execution fee refund logic where the wrong deposit ID (counter instead of depositId) is used to determine the refund recipient and amount. This can result in refunds being sent to the wrong users or being lost entirely.

Vulnerability Details

The bug occurs in the _handleReturn function where the execution fee refund logic incorrectly uses counter instead of the actual depositId of the withdrawal:

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData; // Correct depositId retrieved from flowData
// ... withdrawal logic ...
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // BUG: Using counter instead of depositId
depositInfo[counter].executionFee - usedFee // BUG: Using counter instead of depositId
) {} catch {}
}
}
}

  • Initial withdraw function stores depositId in flowData

  • _handleReturn correctly retrieves depositId from flowData

  • But the refund logic incorrectly switches to using the global counter variable

Impact

  1. Execution fee refunds may be sent to the wrong users (owners of the latest deposit instead of the withdrawing deposit)

  2. Incorrect refund amounts may be calculated and sent

  3. Refunds may fail entirely if depositInfo[counter] has been cleared

  4. Users withdrawing their deposits may lose their execution fee refunds

  5. Unauthorized users may receive refunds they're not entitled to

Tools Used

  • Manual code review

Recommendations

Fix the refund logic in _handleReturn to consistently use the correct depositId:

if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[depositId].owner, // Fix: Use correct depositId
depositInfo[depositId].executionFee - usedFee // Fix: Use correct depositId
) {} catch {}
}
}
Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_counter_invalid_during_handleReturn

Likelihood: Medium/High, when withdraw on a 1x vault. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

Support

FAQs

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