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

nextAction Not Being Reset Properly

Summary

The _withdraw() function in the PerpetualVault.sol sets nextAction when handling withdrawals but does not properly reset it in all cases. This can lead to unexpected transactions, including unintended swaps, if nextAction is executed later without proper reinitialization.

Vulnerability Details

Affected Function: _withdraw()

Code Snippet:

nextAction.selector = NextActionSelector.SWAP_ACTION;
nextAction.data = abi.encode(swapAmount, false);
  • If _withdraw() determines that a swap is needed, it sets nextAction.selector to SWAP_ACTION.

  • However, there is no explicit reset of nextAction in case of an error, manual cancellation, or when _handleReturn() is called directly.

  • If the function exits early or another operation is triggered before execution, nextAction remains set, causing unintended actions when executeNextAction() is later called.

Potential Scenarios Leading to Unexpected Behavior:

  1. Delayed Execution:

    • A user withdraws but does not complete the action chain.

    • nextAction remains in memory and executes when another function triggers executeNextAction().

  2. Unintended Swap Execution:

    • If another user interacts with the contract while nextAction is set, they may inadvertently execute a swap that was never intended for them.

  3. Exploitable State Pollution:

    • Malicious actors could manipulate the contract state by triggering _withdraw(), intentionally leaving nextAction set, and then waiting for another user to unknowingly execute it.

Impact

  • User Funds at Risk: Swaps can be unintentionally executed, resulting in unwanted asset conversions.

  • State Corruption: If nextAction persists between different user transactions, it can cross-contaminate actions between different users.

  • Exploitable Attack Vector: Attackers could force the execution of incorrect swaps, affecting liquidity and trading strategies.

Proof of Concept (PoC)

A test case demonstrating the issue:

function testNextActionPersistence() public {
uint256 depositId = 1;
bytes memory metadata = abi.encode(1000); // Sample metadata for price
// Step 1: Simulate withdrawal where `nextAction` is set
perpetualVault._withdraw(depositId, metadata, marketPrices);
// Step 2: Simulate another user interacting with `executeNextAction()`
vm.prank(attacker); // Simulating another user
perpetualVault.executeNextAction();
// Step 3: Verify if an unintended swap occurred
assertEq(perpetualVault.nextAction.selector, bytes4(0), "nextAction should have been reset!");
}

Tools Used

Manuel, Foundry

Recommendations

Recommendations

  1. Explicitly Reset nextAction after every execution:

    nextAction.selector = bytes4(0);
    nextAction.data = "";

    This ensures nextAction does not persist unexpectedly.

  2. Validate nextAction Before Execution

    • Add a require check to verify that nextAction belongs to the caller before executing.

  3. Introduce a Cancellation Mechanism

    • Allow users to explicitly cancel their nextAction if they do not wish to proceed.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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

Give us feedback!