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

Unnecessary Execution Fee Charges After Liquidation Lead to User Fund Loss

krisrenzo

Summary

The PerpetualVault contract incorrectly charges users execution fees for withdrawals after liquidation, even though no GMX interaction is required. These fees are never refunded, resulting in unnecessary user fund loss.

Vulnerability Details

When a position in the PerpetualVault is liquidated, the contract enters a state where:

  1. The position is closed (curPositionKey is zeroed)

  2. All index tokens are swapped back to collateral tokens

  3. The vault only holds collateral tokens waiting to be withdrawn by users (note that depositPaused is set to true)

However, when users call withdraw() to claim their share of the remaining collateral tokens, they are still charged an execution fee meant for GMX interactions, even though no such interaction is needed. This execution fee is never refunded.

Flow diagram of the issue:

Liquidation occurs
└─► Position closed, tokens swapped to collateral
└─► User calls withdraw()
├─► Charged execution fee unnecessarily
└─► Fee never refunded despite no GMX interaction

Proof of Concept

The issue occurs in this sequence:

  1. After afterLiquidationExecution() is called:

function afterLiquidationExecution() external {
depositPaused = true;
if (sizeInTokens == 0) {
delete curPositionKey;
}
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}

  1. Then runNextAction() processes the FINALIZE action:

function runNextAction(MarketPrices memory prices, bytes[] memory metadata) external {
// ...
if (_nextAction.selector == NextActionSelector.FINALIZE) {
if (IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD) {
// Swap remaining index tokens to collateral
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
_finalize(_nextAction.data);
}
}
​
function _finalize(bytes memory data) internal {
// Enters else block because flow != FLOW.WITHDRAW
delete swapProgressData;
delete flowData;
delete flow;
}

  1. Users calling withdraw() are charged execution fee:

function withdraw(address recipient, uint256 depositId) public payable {
// ...
_payExecutionFee(depositId, false); // Fee charged here
if (curPositionKey != bytes32(0)) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
_settle();
} else { // we enter this else block which requires no GMX interaction
MarketPrices memory prices;
_withdraw(depositId, hex'', prices);
}
}

  1. In _withdraw(), despite taking the liquidated path, fee is not refunded:

function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
// ...
} else if (curPositionKey == bytes32(0)) { // vault liquidated
_handleReturn(0, true, false); // Note the false for refundFee
}
// ...
}

Impact Details

Users lose funds by paying unnecessary execution fees that are never refunded. This affects all users trying to withdraw after liquidation. The impact is amplified in scenarios with many users withdrawing post-liquidation.

Recommendations

Ensure fee refund in the liquidated withdrawal path:

function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
// ...
} else if (curPositionKey == bytes32(0)) { // vault liquidated
- _handleReturn(0, true, false);
+ _handleReturn(0, true, true); // Changed false to true for refundFee
}
// ...
}

Although a preferable approach will to prevents unnecessary fee collection in the first place.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

invalid_withdraw_positionIsClosed_does_not_refund_fees

No fee needed in _payExecutionFee when position is closed. Make a PoC if you disagree.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.