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

Deposit Flow Not Finalized Properly, Blocking Deposits and Withdrawals

Summary

In the PerpetualVault contract, after each flow completes, the flow variable is expected to reset to FLOW.NONE. However, in a specific deposit flow where:

  1. deposit() is called when positionIsClosed == false.

  2. runNextAction() executes.

  3. _runSwap() is triggered due to _isLongOneLeverage(_isLong) and _nextAction.selector == NextActionSelector.INCREASE_ACTION.

  4. _mint() is called when metadata.length == 1, _protocol == PROTOCOL.DEX, and flow == FLOW.DEPOSIT.

The flow is not finalized after _mint(), meaning flow remains FLOW.DEPOSIT instead of being reset to FLOW.NONE. This blocks further deposit and withdrawal functions, creating a stuck state. Additionally, calling cancelFlow() in this state leads to an unintended double refund of the execution fee, as the fee was already refunded during _mint().

Vulnerability Details

  • The deposit process does not complete correctly when _mint() is executed in _runSwap().

  • flow remains stuck at FLOW.DEPOSIT, preventing new deposits or withdrawals.

  • If cancelFlow() is called, it refunds the execution fee again, even though _mint() has already issued a refund of the remaining execution fee.

Impact

  • Users cannot deposit or withdraw funds due to the stuck flow state.

  • Execution fee is refunded twice if cancelFlow id called, leading to unintended fund distribution.

  • Potential for denial-of-service (DoS) attack by repeatedly triggering this condition.

POC

paste in /test/PerpetualVault.t.sol

function test_DepositTxNotFinalized() external {
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
depositFixture(alice, 1e10);
MarketPrices memory prices = mockData.getMarketPrices();
bytes memory paraSwapData = mockData.getParaSwapData(vault);
bytes[] memory swapData = new bytes[](1);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
vm.prank(keeper);
PerpetualVault(vault).run(true, true, prices, swapData);
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 amount = 1e10;
deal(address(collateralToken), alice, amount);
deal(alice, 1e18);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
vm.startPrank(alice);
collateralToken.approve(vault, amount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(
amount
);
vm.stopPrank();
prices = mockData.getMarketPrices();
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault)
.nextAction();
console.log(uint8(selector), 0);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, swapData);
assertEq(uint8(PerpetualVault(vault).flow()), 1);
}

Recommendations

Ensure flow Resets to FLOW.NONE After _mint()by calling _finalize.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_deposit_1x_long_dex_positionIsOpened_DoS_Flow

Likelihood: Medium/High, - Leverage = 1x - beenLong = True - positionIsClosed = False - Metadata → 1 length and Dex Swap Impact: Medium/High, DoS on any new action before the admin uses setVaultState Since this seems to be the most probable path for a 1x PerpVault, this one deserves a High.

Support

FAQs

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

Give us feedback!