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

Using `counter` variable instead of `depositId` in fee refund logic leads to incorrect refunds

anchabadze

Description:

In PerpetualVault.sol, several functions incorrectly use the counter variable instead of the specific depositId when refunding execution fees:

// In _mint()
if (depositInfo[counter].executionFee > usedFee) { // @audit: counter instead of depositId
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // @audit: counter instead of depositId
depositInfo[counter].executionFee - usedFee // @audit: counter instead of depositId
) {} catch {}
}
​
// In _handleReturn()
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // @audit: counter instead of depositId
depositInfo[counter].executionFee - usedFee // @audit: counter instead of depositId
) {} catch {}
}
​
// In _cancelFlow()
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // @audit: counter instead of depositId
depositInfo[counter].executionFee // @audit: counter instead of depositId
) {} catch {}

The counter is a global variable that increments with each new deposit, while depositId represents the specific deposit being processed. Using counter instead of depositId means the refund will be sent to the wrong user.

Example scenario:

// User1 deposits - gets ID 1 (!)
deposit() // counter = 1
// User2 deposits - gets ID 2
deposit() // counter = 2
// User3 deposits - gets ID 3
deposit() // counter = 3 (!)
// User1 wants to withdraw deposit ID 1
withdraw(user1, 1)
​
BUT
​
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, // @audit: counter is 3
depositInfo[counter].executionFee - usedFee // @audit: counter is 3
) {} catch {}
​
​
// User3 gets refund because counter points to deposit 3, but it was User1 who wants to withdraw

Impact:

  • Execution fees being refunded to wrong users

  • Some users never receiving their refunds

  • Loss of user funds through incorrect fee distribution

Recommended Mitigation:

Replace all instances of counter with the correct depositId

Updates

Lead Judging Commences

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

finding_counter_invalid_during_cancelFlow_after_withdrawing

Likelihood: Low, contract has to call cancelFlow after a withdraw, and the settle action is already executed by GMX. Impact: High, the fees will be distributed to the last depositor and not the withdrawer.

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.