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

Execution Fee Refund Fails Due to Premature Deposit Deletion

Summary

When a user withdraws their deposit, the contract burns(_burn()) the depositId, deleting its associated data before attempting to process the execution fee refund. As a result, depositInfo[depositId].executionFee no longer exists when the refund logic runs causing the refund to fail silently, and the user loses their execution fee.

Vulnerability Details

In the handleReturn() function, the contract first deletes the deposit (_burn(depositId)) and then tries to access depositInfo[depositId] is already deleted, its values are reset to zero due to Solidity's default mapping behavior.

This results in

  • depositInfo[depositId].executionFee = 0, making the refund impossible.

  • Th try/catch block suppresses any error, leading to a silent failure.

  • The user never gets back unused execution fee.

emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId); // deletes deposit info & Id
// No way to get refunded if no deposit Id & info occurs
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
}

Impact

Direct financial loss for users

Tools Used

Manual Review

Recommendations

To correct this issue the refund logic should be executed before _burn(depositId), ensuring the contract still has access to the execution fee.

if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee) {} catch {}
}
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_burn(depositId); // Now it's safe to delete deposit info
Updates

Lead Judging Commences

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