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

Deposit during an active 1x leveraged long position can result in a complete DoS of the protocol due to a Stalling Flow State

Summary

The Perpetual Vault contract enables 1x Long positions by swapping the collateral token deposited by users into the index token. These swaps are executed using ParaSwap and GMX. To facilitate these transactions, the PerpetualVault contract includes a _runSwap function that takes a bytes array containing swap data, the swap direction, and the current prices of the market tokens. The swap data is generated off-chain and may contain only ParaSwap data, GMX swap data, or both as per the code natspec.

Vulnerability Issues

The problem arises when a user deposits collateral while a 1x Long position is active. The protocol is designed to add the new collateral to the existing position. This process is managed via a keeper mechanism. The nextAction.selector is set toINCREASE_ACTION, then the keeper calls runNextAction, the function invokes _runSwap to swap the newly added collateral into the index token.

The issue occurs if the swap data includes only ParaSwap data. The collateral swap is executed using _doDexSwap, but the flow variable is not reset after the transaction, leading to an inconsistent state.

Faulty Code Block:

PerpetualVault.sol#L1003-L1006

Steps to Reproduce

  1. The vault has an active long position with 1x leverage.

  2. A user deposits collateral into the vault, flow is set to DEPOSIT.

  3. The keeper generates swap data that only involves swapping via _doDexSwap.

  4. The swap takes place, but the flow does not get reset as intended.

  5. This leads to the loss of major protocol functionality.

Impact

This vulnerability results in a complete denial-of-service (DoS) for critical protocol functions such as:

  • Deposits

  • Withdrawals

  • Opening / Closing positions

  • Claiming collateral rebates

Proof of Concept (PoC)

The following test case demonstrates the issue using the provided ParaSwap swap data in the test suite:

function test_Run_Paraswap_during_active_position_deposit() external {
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
depositFixture(alice, 1e10);
// Alice deposits collateral, and the vault opens a 1x Long Position
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);
address bob = makeAddr("bob");
deal(bob, 1 ether); // Cover execution fees
depositFixture(bob, 1e10);
// Bob's deposit sets nextAction to INCREASE_ACTION, and the keeper calls runNextAction
prices = mockData.getMarketPrices();
paraSwapData = mockData.getParaSwapData(vault);
swapData = new bytes[](1);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, swapData);
assertEq(uint8(PerpetualVault(vault).flow()), 1); // The DEPOSIT flow is never reset
}

This test case confirms that after the swap is completed, the flow state remains DEPOSIT (1), preventing further protocol actions.

enum FLOW {
NONE,
DEPOSIT, // 1
SIGNAL_CHANGE,
WITHDRAW,
COMPOUND,
LIQUIDATION
}

Tools Used

  • Manual Review

  • Foundry

Recommendations

To resolve this issue, call the _finalize() function at the end of the deposit flow during a swap that takes place via _runSwap when ParaSwap is the sole DEX used. This resets the global state variables and allows the protocol to continue operating as intended.

Suggested Fix:

function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices)
internal
returns (bool completed)
{
[...]
else {
if (metadata.length != 1) {
revert Error.InvalidData();
}
(PROTOCOL _protocol, bytes memory data) = abi.decode(metadata[0], (PROTOCOL, bytes));
if (_protocol == PROTOCOL.DEX) {
uint256 outputAmount = _doDexSwap(data, isCollateralToIndex);
// update global state
if (flow == FLOW.DEPOSIT) {
// last `depositId` equals with `counter` because another deposit is not allowed before previous deposit is completely processed
_mint(counter, outputAmount + swapProgressData.swapped, true, prices);
+ _finalize(hex"");
}
[...]
}
}
}
Updates

Lead Judging Commences

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