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

Wrong Fee Refund Logic During Withdrawal and Cancel Flow Leads to Improper Fee Refunds

riceee

Summary

The PerpetualVault contract includes logic for processing withdrawals or to cancel an onging withdrawal flow and refund unused execution fees to users. However, the parameters used for IGmxProxy(gmxProxy).refundExecutionFee to refund fees leads to execution fees refunded to the wrong user and also results in incorrect refund amounts.

Vulnerability Details

In the withdrawal flow and during cancelling a withdrawal flow, the contract attempts to refund execution fees, but it incorrectly references depositInfo[counter] instead of depositInfo[depositId] / depositInfo[flowData] respectively. Since counter might point to the latest depositor Id (unless last action was a cancelFlow during deposit), this leads to:

  • Execution fees being refunded to the wrong user.

  • Incorrect refund amounts being issued.

Affected Code:

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 {}
}
}

In _cancelFlow function

else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee) {} <@
catch {}
}

Impact

  • Users will not receive their execution fee refunds.

  • Refunds may be incorrectly allocated to other users.

Tools Used

  • Manual Review

Recommendations

Update the contract to ensure that the correct depositId is used when referencing execution fee refunds.

Suggested Fix:

if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
+ depositInfo[depositId].owner,
+ depositInfo[depositId].executionFee - usedFee
) {} catch {}
}
}
else if (flow == FLOW.WITHDRAW) {
+ try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[flowData].owner, depositInfo[flowData].executionFee) {}
catch {}
}
Updates

Lead Judging Commences

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

Appeal created

riceee Submitter
5 months ago
n0kto Lead Judge
5 months ago
n0kto Lead Judge 4 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.