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

Incorrect Deposit ID in Execution Fee Refund

0x68

Bug Report: Incorrect Deposit ID in Execution Fee Refund

Summary

There is a critical vulnerability in the PerpetualVault contract where execution fee refunds are processed using the wrong deposit ID (counter instead of depositId), which could lead to refunds being sent to incorrect users or transactions failing.

Vulnerability Details

The vulnerability appears in two locations:

// Location 1: In _mint() function
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
// ^^^^^^^ Should be depositId
}
}
​
// Location 2: In _handleReturn() function
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
// ^^^^^^^ Should be depositId
}
}

Key issues:

  1. Uses global counter variable instead of the specific depositId

  2. counter increments with each new deposit

  3. depositId and counter may reference different deposits

Impact

  1. Wrong Recipient: Execution fee refunds could be sent to incorrect users

  2. Lost Funds: Users might not receive their rightful refunds

  3. Failed Transactions: If counter references a non-existent deposit

  4. Financial Loss: Potential for users to lose execution fees

  5. Trust Issues: Incorrect fund distribution affects protocol reliability

Tools Used

  • Manual code review

  • Static analysis

Recommendations

Short Term Fix

Replace counter with depositId in both locations:

// In _mint() and _handleReturn()
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[depositId].owner, // Fix: Use depositId instead of counter
depositInfo[depositId].executionFee - usedFee
) {} catch {}
}
}

Long Term Fix

  1. Add input validation for deposit IDs

  2. Implement an event for fee refunds

  3. Consider adding a dedicated fee tracking system

  4. Add test cases specifically for execution fee refund scenarios

Updates

Lead Judging Commences

n0kto Lead Judge 3 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.