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

Withdrawal Flow Cancellation Fails (Reverts) Due to Missing Data in Finalization Step

Vulnerability Details

In the PerpetualVault contract, there is a critical issue in the withdrawal cancellation flow that causes the entire operation to fail. When a withdrawal operation is initiated while an open position exists, the contract follows a specific sequence: settling outstanding fees, updating the position state, and queuing the operation for later execution by the keeper. For the flow to be canceled, the order is canceled first by calling cancelOrder() , and then the flow is canceled by calling cancelFlow().

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(userDeposits[depositInfo[depositId].owner], depositId);
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
delete depositInfo[depositId];
} else if (flow == FLOW.WITHDRAW) {
try IGmxProxy(gmxProxy).refundExecutionFee(
depositInfo[counter].owner,
depositInfo[counter].executionFee
) {} catch {}
}
// Setting flow to liquidation has no meaning.
// The aim is to run FINAIZE action. (swap indexToken to collateralToken);
flow = FLOW.LIQUIDATION;
>>> nextAction.selector = NextActionSelector.FINALIZE;
}
function _finalize(bytes memory data) internal {
if (flow == FLOW.WITHDRAW) {
>>> (uint256 prevCollateralBalance, bool positionClosed, bool refundFee) = abi.decode(data, (uint256, bool, bool));
uint256 withdrawn = collateralToken.balanceOf(address(this)) - prevCollateralBalance;
_handleReturn(withdrawn, positionClosed, refundFee);
} else {
delete swapProgressData;
delete flowData;
delete flow;
}
}

The root cause lies in the implementation of the cancellation flow. When the withdrawal needs to be canceled, the _cancelFlow() function is called, which sets the flow state to LIQUIDATION and updates nextAction.selector to FINALIZE. However, it critically fails to set the required data in NextAction that the _finalize() function expects to decode. This oversight causes the finalization step to revert when the keeper attempts to execute the queued action through runNextAction().

Impact

This implementation flaw prevents the proper completion of withdrawal cancellations, potentially leaving the contract in an inconsistent state and affecting user operations. Also, suppose a malicious user tries to execute some malicious operation, the keeper will be unable to halt its execution seeing as the flow cannot be cancelled in this case.

Proof of Concept

  1. A user initiates a withdrawal with an open position

  2. Contract processes initial withdrawal steps:

    • Settles outstanding fees

    • Updates position state

    • Adds withdrawal to nextAction queue

  3. Keeper attempts to cancel the withdrawal:

    • Calls cancelOrder()

    • Calls _cancelFlow()

    • Flow state changes to LIQUIDATION

    • nextAction.selector is set to FINALIZE

    • No data is set for finalization

  4. Keeper calls runNextAction()

  5. _finalize() attempts to decode non-existent data

  6. Transaction reverts, leaving cancellation incomplete

Recommendation

Modify the _cancelFlow() function to properly set the required data for finalization:

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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