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

Burning user deposit information before verifying their eligibility for a fee refund will always result in no fees being refunded in PerpetualVault::__handleReturn

riceee

Vulnerability Details

When processing withdrawals, _handleReturn is executed at the end of the withdrawal flow to transfer collateral back to the user proportionally to their share. This function includes a bool refundFee parameter to determine whether the user is eligible for a refund on execution fees paid during the withdraw function call.

However, before verifying the refund eligibility, the function prematurely burns the depositorInfo. As a result, even if a user qualifies for a fee refund, they will never receive it. This occurs because if (depositInfo[depositId].executionFee > usedFee) will always evaluate to false since the relevant deposit information has already been deleted resulting in depositInfo[depositId].executionFee to always be zero.

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
[...]
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
_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 {}
}
}
​
// update global state
delete swapProgressData;
delete flowData;
delete flow;
}

Impact

If the user was supposed to get a refund on the execution fees they paid, for example, during withdrawing from a 1x leveraged long position, they ultimately end up losing the entire execution fees for that transaction.

Tools Used

Manual Review

Recommendations

_burn should be called after the refund functionality

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
​
[...]
​
emit Burned(depositId, depositInfo[depositId].recipient, depositInfo[depositId].shares, amount);
- _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);
​
// update global state
delete swapProgressData;
delete flowData;
delete flow;
}
Updates

Lead Judging Commences

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