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 {}
}
}
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.