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

If Vault Was Liquidated And There Was A Withdrawal Flow Then Fee Should Be Refunded

sakshamseth5

Summary

The issue in short is that when a user has requested a withdraw and the vault gets liquidated with sizeInTokens as 0 , then there would not be a decrease order made on GMX (because of liquidation) and the user would be paid from whatever was received after liquidation and the user's shares , in this case the user should be refunded for the fees since there was no GMX order made , but we will see how there was no refund made to the user.

Vulnerability Details

Consider the following ->

1.) There is an active perp vault position on GMX with leverage > 1x.

2.) A user has requested a withdraw using withdraw() and pays the execution fee ->

[https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/PerpetualVault.sol#L272]

the flow is assigned as WITHDRAW at L255 , and since if (curPositionKey != bytes32(0)) (cause a position is open on GMX with leverage > 1x) ->

if (curPositionKey != bytes32(0)) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
_settle(); // Settles any outstanding fees and updates state before processing withdrawal
}

Therefore , next action is WITHDRAW_ACTION and _settle() is called.

3.) Inside _settle() a settle order is created (routed through GmxProxy.sol) ->

function _settle() internal {
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
market: market,
indexToken: indexToken,
initialCollateralToken: address(collateralToken),
swapPath: new address[](0),
isLong: beenLong,
sizeDeltaUsd: 0,
initialCollateralDeltaAmount: 0,
amountIn: 0,
callbackGasLimit: callbackGasLimit,
acceptablePrice: 0,
minOutputAmount: 0
});
_gmxLock = true;
gmxProxy.settle(orderData);
}

4.) After a successful settle order , afterOrderExecution() would be invoked by GmxProxy and nextAction would be assigned as WITHDRAW_ACTION ->

if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
emit GmxPositionCallbackCalled(requestKey, true);
return;
}

5.) Now lets say the position in GMX got fully liquidated , therefore afterLiquidationExecution() would be invoked (L563) ->

function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
​
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
}
​
if (flow == FLOW.NONE) {
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
} else if (flow == FLOW.DEPOSIT) {
flowData = sizeInTokens;
} else if (flow == FLOW.WITHDRAW) {
// restart the withdraw flow even though current step is FINALIZE.
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
}

and since sizeInTokens would be 0 (fully liquidated ) then curPositionKey would be deleted (would be 0 now) and since flow was WITHDRAW , nextAction.selector would be assigned WITHDRAW_ACTION

6.) Then keeper would invoke runNextAction() and since nextAction is WITHDRAW_ACTION , this branch would be invoked (L371-L381)->

else if (_nextAction.selector == NextActionSelector.WITHDRAW_ACTION) {
// swap indexToken that could be generated from settle action or liquidation/ADL into collateralToken
// use only DexSwap
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
uint256 depositId = flowData;
_withdraw(depositId, metadata[0], prices);

Therefore _withdraw() is invoked.

7.) And inside withdraw , this branch would be invoked since curPositionKey = 0 (L1102) , ->

} else if (curPositionKey == bytes32(0)) { // vault liquidated
_handleReturn(0, true, false);
}

8.) In the above _handleReturn call refundFee parameter has been set to false (3rd parameter) , but this is incorrect , since in this case no decrease orders were opened in GMX (which happen in the last else case in _withdraw) and therefore the user should have been refunded the fee.

9.) Therefore after the collateral is transferred in _handleReturn() there was no refund which is wrong as explained above.

Impact

In case the position got liquidated , there was no decrease order made on GMX , hence the user should not have been charged the fee and should have been given a refund , but in the above flow we see user will not receive a refund when vault is liquidated.

Tools Used

Manual analysis

Recommendations

Instead do

} else if (curPositionKey == bytes32(0)) { // vault liquidated
_handleReturn(0, true, true);
}
Updates

Lead Judging Commences

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

Appeal created

sakshamseth5 Submitter
5 months ago
n0kto Lead Judge
5 months ago
n0kto Lead Judge 4 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_liquidation_before_withdrawing_order_does_not_refund_fees

Likelihood: Low. A decrease order has to be executed just after a liquidation. Impact: Medium. Execution fees are not refunded, even if not used.

Support

FAQs

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