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

`nextAction` is not cleared if signal change order is cancelled.

pkqs90

Summary

nextAction is not cleared if signal change order is cancelled. This breaks one of the key invariants of the protocol.

Vulnerability Details

There is a key invariant in the contest readme that when an action is cimplete, the nextAction is empty. I'm guessing the there is a bot that monitors the nextAction variable for calls to the keeper contract which triggers the runNextAction() function.

After all actions completed, nextAction, swapProgressData should be empty. PositionKey is 0 when no position

However, when conducting a signal change by run(), if the position is not 1xLong, it needs to first close the current position, and open a new one. The nextAction.selector is set to INCREASE_ACTION.

If the decreasePosition order for closing position fails and gets cancelled, it will enter the afterOrderCancellation() function. The bug here is, since we are in signal_change flow, it clears the flow and flowData, but does not clear nextAction.

This breaks one of the key invariants of this protocol, and will confuse the keeper.

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

function run(
bool isOpen,
bool isLong,
MarketPrices memory prices,
bytes[] memory metadata
) external nonReentrant {
...
// 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);
}
...
}
​
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.
@> // @audit-bug: nextAction is not cleared.
delete flowData;
delete flow;
}
}
emit GmxPositionCallbackCalled(requestKey, false);
}

Impact

This breaks one of the key invariants of this protocol, and will confuse the keeper.

Tools Used

N/A

Recommendations

Also clear nextAction.

Updates

Lead Judging Commences

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