DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: high
Valid

Incorrect value passed to `GmxProxy::refundExecutionFee` in `_handleReturn`

Summary

In Perpetual::_handleReturn, at the end, if there is refund parameter set to true, refund will be processed, and correspond value will be passed in as argument to refundExecutionFee to proceed refund. However, the current implementation passes incorrect value, causing refund processed incorrectly.

Vulnerability Details

Here at the end of _handleReturn:

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

If execution fee is greater than used fee, meaning there is excess fee to be refunded, however, in refundExecutionFee, the depositInfo is getting index counter, instead of depositId, original user will not receive refund, but the last deposit owner will.

Impact

Refund is handled incorrectly, intended user will not receive refund.

Tools Used

Manual review

Recommendations

For index in depositInfo, use depositId instead.

Updates

Lead Judging Commences

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

Give us feedback!