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 10 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.

Give us feedback!