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

When Flow Was SIGNAL_CHANGE And The Decrease Order Gets Cancelled , nextAction Is Not Deleted Breaking An Invariant

sakshamseth5

Summary

In the case where current position is being closed and a new position is being opened , if the market decrease order fails to close the current position , then the close order hook in perp vault (afterOrderCancellation()) does not reset the nextAction breaking a core invariant in the README.

Vulnerability Details

Consider the following (this flow has been confirmed by the sponsor) ->

1.) There's a perp vault position open on GMX with leverage > 1x. This position got liquidated and the perp vault received the remaining amount after liquidation from GMX.

2.) This position is being closed and a new one will be opened ( reason can be anything for ex - after liquidation etc the current position will be closed and a new one will be opened) , for that the keeper would call the run() function with isOpen as true and currently positionIsClosed is false (since positionIsClosed is not updated on liquidation in afterLiquidationExecution()) , therefore this branch in the run() function gets executed (L314-L325) ->

else {
if (beenLong == isLong) {
revert Error.NoAction();
} else {
// Close current position first and then open the requested position in the next action
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(isLong);
if (_isLongOneLeverage(beenLong)) {
_runSwap(metadata, false, prices);
} else {
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createDecreasePosition(0, 0, beenLong, acceptablePrice, prices);
}

what this does is , closes the current position and requests to open a position in next action , therefore we see that ->

nextAction.selector = NextActionSelector.INCREASE_ACTION;

and , since leverage > 1x , _createDecreasePosition will be called with 0 values to denote current position is being closed.

Also the flow is assigned to SIGNAL_CHANGE at L301.

3.) In _createDecreasePosition() a market order of type MarketDecrease is created ->

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

4.) For some reason this was not executed on GMX and now this order needs to be cancelled in perp vault , therefore the keeper would call cancelOrder()
, and after the order is cancelled the afterOrderCancellation() hook will be called in perp vault ->

function afterOrderCancellation(
bytes32 requestKey,
Order.OrderType orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
​
if (orderResultData.isSettle) {
// Retry settle action.
nextAction.selector = NextActionSelector.SETTLE_ACTION;
} else if (orderType == Order.OrderType.MarketSwap) {
// If GMX swap fails, retry in the next action.
nextAction.selector = NextActionSelector.SWAP_ACTION;
// abi.encode(swapAmount, swapDirection): if swap direction is true, swap collateralToken to indexToken
nextAction.data = abi.encode(swapProgressData.remaining, swapProgressData.isCollateralToIndex);
} else {
if (flow == FLOW.DEPOSIT) {
nextAction.selector = NextActionSelector.INCREASE_ACTION;
nextAction.data = abi.encode(beenLong);
} else if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
} else {
// If signal change fails, the offchain script starts again from the current status.
delete flowData;
delete flow;
}
}

We can see that if flow = SIGNAL_CHANGE then the else block at the end would be triggered i.e.

else {
// If signal change fails, the offchain script starts again from the current status.
delete flowData;
delete flow;
}

5.) flowData and flow are deleted , but nextAction has not been deleted and is still assigned to INCREASE_ACTION breaking the invariant mentioned in the README After all actions completed, nextAction, swapProgressData should be empty , this is because nextAction is supposed to be registered off-chain for actions and will be completed on-chain by keepers , therefore , nextAction is incorrect.

Impact

nextAction has not been deleted and is still assigned to INCREASE_ACTION breaking the invariant mentioned in the README After all actions completed, nextAction, swapProgressData should be empty

Tools Used

Manual analysis

Recommendations

Delete the nextAction also if order cancelled.

Updates

Lead Judging Commences

n0kto Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid_afterOrderCancellation_do_not_reset_nextAction

Normal behavior, the keeper will retry to increase the position. That’s why there is this condition in `_createIncreasePosition`: ``` if (flow == FLOW.DEPOSIT) { amountIn = depositInfo[counter].amount; flowData = vaultReader.getPositionSizeInTokens(curPositionKey); } else { amountIn = collateralToken.balanceOf(address(this)); } ```

Appeal created

sakshamseth5 Submitter
5 months ago
n0kto Lead Judge
5 months ago
n0kto Lead Judge 4 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

invalid_afterOrderCancellation_do_not_reset_nextAction

Normal behavior, the keeper will retry to increase the position. That’s why there is this condition in `_createIncreasePosition`: ``` if (flow == FLOW.DEPOSIT) { amountIn = depositInfo[counter].amount; flowData = vaultReader.getPositionSizeInTokens(curPositionKey); } else { amountIn = collateralToken.balanceOf(address(this)); } ```

Support

FAQs

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