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

`refundExecutionFee` is never executed in `_handleReturn`

Summary

refundExecutionFee() is never executed in _handleReturn().

Vulnerability Details

In PerpetualVault::_handleReturn(), _burn() is called before refunding fees, in _burn(), the depositInfo[depositId] is deleted, which means depositInfo[depositId].executionFee will be 0 and the if condition is always false, so refundExecutionFee() will never be executed.

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
(uint256 depositId) = flowData;
uint256 shares = depositInfo[depositId].shares;
uint256 amount;
if (positionClosed) {
amount = collateralToken.balanceOf(address(this)) * shares / totalShares;
} else {
uint256 balanceBeforeWithdrawal = collateralToken.balanceOf(address(this)) - withdrawn;
amount = withdrawn + balanceBeforeWithdrawal * shares / totalShares;
}
if (amount > 0) {
_transferToken(depositId, amount);
}
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;
}
function _burn(uint256 depositId) internal {
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
totalShares = totalShares - depositInfo[depositId].shares;
@> delete depositInfo[depositId];
}

Impact

Execess execution fee is never refunded to user in the withdraw flow.

Tools Used

Manual.

Recommendations

burn depositId after refundExecutionFee.

Updates

Lead Judging Commences

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