DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: medium
Invalid

[M-02] Continuous Failures in `afterOrderCancellation()` Can Prevent Proper Order Recovery

Summary

The afterOrderCancellation() function in the contract PerpetualVault.sol is responsible for handling failed order executions on GMX. However, the function does not properly recover from persistent failures, meaning that if an order fails once, it may continue to fail indefinitely, preventing the vault from progressing past the failed state. This issue arises due to the function blindly retrying the same failed action without considering why it failed in the first place.

Vulnerability Details

The function attempts to retry failed operations by setting nextAction.selector to retry swaps, deposits, or withdrawals. However, if the underlying reason for failure is not resolved, the same action will continue to be retried, causing an infinite loop of failures.

Relevant code snippet:

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;
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;
}
}
emit GmxPositionCallbackCalled(requestKey, false);
}

Issue Explanation

  • Blind Retries:

    • The function always sets nextAction.selector to retry the failed action, regardless of the underlying cause of failure.

    • If the issue is due to insufficient liquidity, price impact, or incorrect calldata, retrying does not solve the problem.

  • No Failure Handling Mechanism:

    • If an order fails multiple times, there is no mechanism to halt retries or escalate the failure.

    • This can lock the vault in an infinite failure loop, preventing any further interactions.

  • No Fallback or Escalation Logic:

    • If the issue persists, the contract should escalate the failure, possibly requiring off-chain intervention or logging a critical failure event.

    • The current implementation assumes that retrying is always the correct solution, which is not the case.

Impact

  • Funds may become stuck indefinitely as the contract gets stuck in an infinite loop of failed transactions.

  • Vault operations may halt, as the system keeps retrying the same failed action without resolving the root cause.

  • Possible gas drain if keepers continue submitting transactions that fail due to the same underlying issue.

  • Users may not be able to deposit, withdraw, or modify positions, leading to serious usability issues.

Tools Used

  • Manual Code Review

Recommendations

  1. Introduce a retry limit:

    • Implement a counter for retries, and if an action fails more than N times, log an error and stop retrying.

    • Example fix:

      mapping(bytes32 => uint256) public retryCount;
      uint256 public constant MAX_RETRIES = 3;
      if (retryCount[requestKey] >= MAX_RETRIES) {
      emit CriticalFailure(requestKey, orderType);
      delete nextAction;
      return;
      }
      retryCount[requestKey]++;
  2. Log a critical failure event if an action fails too many times so off-chain monitoring systems can intervene.

    event CriticalFailure(bytes32 requestKey, Order.OrderType orderType);
  3. Add a validation step before retrying orders:

    • Before retrying, check why the order failed (e.g., insufficient balance, invalid calldata, price impact too high).

    • Example:

      if (!vaultReader.isOrderValid(requestKey)) {
      revert Error.OrderCannotBeRetried();
      }
  4. Introduce a fallback mechanism:

    • If an order keeps failing, reset the contract state and require manual intervention instead of retrying indefinitely.

    • Example:

      if (retryCount[requestKey] >= MAX_RETRIES) {
      delete flowData;
      delete flow;
      delete nextAction;
      }

Proposed Fix

Modify afterOrderCancellation() to prevent infinite failures:

function afterOrderCancellation(
bytes32 requestKey,
Order.OrderType orderType,
IGmxProxy.OrderResultData memory orderResultData
) external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
_gmxLock = false;
if (retryCount[requestKey] >= MAX_RETRIES) {
emit CriticalFailure(requestKey, orderType);
delete nextAction;
return;
}
retryCount[requestKey]++;
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.SETTLE_ACTION;
} else if (orderType == Order.OrderType.MarketSwap) {
nextAction.selector = NextActionSelector.SWAP_ACTION;
nextAction.data = abi.encode(swapProgressData.remaining, swapProgressData.isCollateralToIndex);
} else if (orderType == Order.OrderType.MarketDecrease) {
nextAction.selector = NextActionSelector.DECREASE_ACTION;
nextAction.data = abi.encode(beenLong);
} else {
delete flowData;
delete flow;
}
emit GmxPositionCallbackCalled(requestKey, false);
}
Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice

Support

FAQs

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