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

Execution Fees will never get refunded in _handleReturn

Summary

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/PerpetualVault.sol#L1129-L1150

During a withdrawal, the last step of the process is the invocation of PerpetualVault::_handleReturn. This function handles the refund of fees is applicable.
It will, however, never refund any fees.

Vulnerability Details

One of the inputs to _handleReturn is refundFee. If set to true, and the user has paid more execution fees than used, this function should refund the difference:

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

The problem lies in the fact that depositInfo[depositId] is burnt just the line above this condition is checked so depositInfo[depositId].executionFee is always zero and will never meet this condition (if (depositInfo[depositId].executionFee > usedFee)):

_burn(depositId);
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;

Impact

User will never get any fee refunded.

Tools Used

Manual review

Recommendations

Cache the value of depositInfo[depositId] before burning or burn depositId after fees are refunded.

Updates

Lead Judging Commences

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

finding_burn_depositId_before_refund

Likelihood: High, every time a user withdraw on 1x vault with paraswap Impact: Medium, fees never claimed to GMX and refund to the owner.

Support

FAQs

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