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

_handleReturn function cannot refund fee to the owner

Summary

The deposit owner cannot get a refund for the execution fee on withdrawals.

Vulnerability Details

The _handleReturn function is the end of the withdrawal flow, this function calculates the collateral amount that should be sent to the treasury and the user and makes those transfers, then updates the totalShares and deletes the depositInfo mapping by calling the _burn function, after deleting the depositInfo mapping it will try to refund the executionFee to the deposit owner, but the if (depositInfo[depositId].executionFee > usedFee) would never be true because the depositInfo mapping has been reset to default values in the _burn function, so the call to the gmxProxy.refundExecutionFeefunction will never happen and the user will never receive the refund of this executionFee.

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;
}
}

Impact

The deposit owner loses the execution fee refund.

Tools Used

Manual review

Recommendations

call the _burn function after the if (refundFee) has ended.

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.