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

Execution Fee refund via the `_handleReturn` function is never executed

0xodus

Summary

The refundExecutionFee call in the _handleReturn function cannot execute

Vulnerability Details

In the _handleReturn function in the PerpetualVault contract has a conditional statement if (refundFee) which is meant to refund the unused execution fee, however before the conditional statement is reached in the flow of the function, there is a call to the _burn(depositId) function which deletes the deposit Information maping for the depositId as show below

function _burn(uint256 depositId) internal {
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
totalShares = totalShares - depositInfo[depositId].shares;
@> delete depositInfo[depositId];
}
​
function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
// Skipped for simplicty
@> _burn(depositId);
​
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
@> if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
) {} catch {}
}
}
// Skipped for simplicity
}

As shown in the above illustration, after the call to the _burn function that deletes the mapping, and the refundFee bool is true, in the loop the depositInfo[depositId].executionFee statement will always return 0 since the mapping is non-existent, therefore the executionFee will never be > usedFee with is neccesary to trigger the execution fee refund function in the gmxProxy contract.
This will happen when the _handleReturn function is called in the runSwap function. The specific instance is given below:
https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/PerpetualVault.sol#L1007

Impact

  1. Loss of User Funds:
    Users who are entitled to a refund of excess execution fees (when depositInfo[depositId].executionFee > usedFee) will never receive it. The condition depositInfo[depositId].executionFee > usedFee can never evaluate to true post-deletion, causing the refundExecutionFee call to the GmxProxy contract to be skipped. This effectively results in users losing funds they should have reclaimed, undermining trust in the system.

  2. Permanent Underfunding of Refunds :
    The perpetual deletion of the depositInfo[depositId] mapping before the refund logic ensures that no execution fee refunds are processed, regardless of the actual gas usage (usedFee = callbackGasLimit * tx.gasprice). This disrupts the intended economic model of the contract, where unused fees are returned to depositors, potentially leading to an accumulation of unclaimed funds within the system or misaligned incentives.

Tools Used

Manual Review

Recommendations

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
// Skipped for simplicty
- _burn(depositId);
​
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner, depositInfo[counter].executionFee - usedFee
) {} catch {}
}
}
+ _burn(depositId);
// Skipped for simplicity
}
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.