Liquidity Management

Summary

Under an edge case , a withdrawing user would receive more collateral than he is entitled to and due to this other depositors when withdrawing will receive lesser shares leading to a permanent loss of funds due to this.

Vulnerability Details

Consider the following ->

1.) A perp vault position is open on GMX with leverage > 1x.

2.) UserA has requested a withdraw using the withdraw() function , let's assume that there is 100,000 USDC collateral in the position and there are just 2 depositors currently i.e UserA and UserB. Therefore, upon withdrawal UserA must receive 50,000 USDC (not accounting for fee and other things).

And , next action is WITHDRAW_ACTION and _settle() is called ->

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

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

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

5.) Now since nextAction is WITHDRAW_ACTION , keeper would invoke runNextAction() where this branch would be invoked ->

Therefore , _withdraw() is invoked and in _withdraw() this branch would be invoked ->

6.) collateralDeltaAmount is calculated by collateral per user according to the shares of the user , in our case userA had 50% of
the shares therefore it would be 50,000 USDC (not accounting for fee , remember these numbers are just for example and to depict the issue).

Then _createDecreasePosition is called with the collateralDeltaAmount and sizeDeltaInUsd which is calculated similarly , and in
_createDecreasePosition() a market decrease order is created at L936.

7.) When this order runs successfully this branch would be invoked in afterOrderExecution() ->

where prevCollateralBalance is the balance before the output amount was received (output amount is 50,000 USDC in our example) ,
and nextAction is FINALIZE now.

8.) Now , let's assume ADL kicks in and closes the half of the position meaning 25,000 USDC is in the position and 25,000 worth is getting closed, this will happen through afterOrderExecution() in GmxProxy.sol and there this branch would be invoked (L248-L258 in GmxProxy.sol)->

The above code would send 25,000 USDC to the perp vault (according to our example) and our position in GMX is worth 25,000 USDC now(notice
that afterLiquidationExecution would not be called in partial close).

9.) Now , since nextAction is FINALIZE , keeper calls runNextAction() ->

, _finalize() is invoked where ->

10.) This is where the issue arises , since in partial close due to ADL perp vault received 25,000 USDC , the withdrawn value i.e.
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance; would be 25,000 USDC higher , therefore _handleReturn
is called with withdrawn amount 25,000 USDC more than it should be and hence ->

amount to be transferred would be 75,000 + more_profit_due_to_extra_25k (there can be more collateral in the contract due to funding fees etc to make up for the more_profit_due_to_extra_25k)

11.) Therefore , due to this edge case the userA made an excessive profit and all other depositors will incur huge losses now in withdrawals. The root cause was that in _finalize we calculate the withdrawn amount as balanceOf - prevCollateralBalance which increased due to the scenario described.

Impact

UserA got excessive amounts due to not accounting for the received amount correctly , and further withdraws by other depositors will incur losses now which will be permanent loss of funds.

Tools Used

Manual analysis

Recommendations

Only account for the amount that was received when the position was closed , any further amount received by perp vault during when the nextAction was set as FINALIZE and actual finalization would be sent to the user withdrawing as a whole.