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

It is impossible to refund the fee to the user due to logic flaw in the _handleReturn function

biakia

Summary

The _handleReturn function in the PerpetualVault contract is responsible for processing withdrawals and refunding execution fees to users. However, there is a critical issue in the order of operations: the function calls _burn before attempting to refund the execution fee. Since _burn deletes depositInfo[depositId], the execution fee stored in depositInfo[depositId].executionFee is lost, making it impossible to refund the fee to the user. This results in financial loss for users and a broken refund mechanism.

Vulnerability Details

The _handleReturn function processes withdrawals and attempts to refund the execution fee if refundFee is true. However, it calls _burn before the refund logic, which deletes depositInfo[depositId].

function _handleReturn(uint256 withdrawn, bool positionClosed, bool refundFee) internal {
...
...
_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;
}

The _burn function deletes depositInfo[depositId], which includes the executionFee field.

function _burn(uint256 depositId) internal {
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
totalShares = totalShares - depositInfo[depositId].shares;
delete depositInfo[depositId];
}

As a result, the execution fee stored in depositInfo[depositId].executionFee is lost, making it impossible to refund the fee to the user. This results in financial loss for users and a broken refund mechanism.

Impact

  • Execution Fee Loss: When _burn is called, depositInfo[depositId] is deleted, including the executionFee field. As a result, the execution fee is permanently lost, and the refund logic cannot access it.

  • Broken Refund Mechanism: The refund mechanism is effectively broken because depositInfo[depositId].executionFee is no longer available when the refund logic is executed.

The impact is High, the likelihood is High, so the severity is High.

Tools Used

Maunal Review

Recommendations

To fix this issue, the order of operations in _handleReturn should be adjusted so that the execution fee is refunded before _burn is called. This ensures that depositInfo[depositId].executionFee is still accessible when the refund logic is 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);
​
// Refund execution fee BEFORE calling _burn
if (refundFee) {
uint256 usedFee = callbackGasLimit * tx.gasprice;
if (depositInfo[depositId].executionFee > usedFee) {
try IGmxProxy(gmxProxy).refundExecutionFee(depositInfo[depositId].owner, depositInfo[depositId].executionFee - usedFee) {} catch {}
}
}
​
_burn(depositId); // Call _burn AFTER refunding the execution fee
​
// Update global state
delete swapProgressData;
delete flowData;
delete flow;
}
​
Updates

Lead Judging Commences

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