Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

The entire protocol can be subjected to a DoS attack when swapping collateral tokens through `Paraswap`.

bizarro

Summary

In PerpetualVault contract, users can deposit collateral tokens to receive shares and later withdraw their collateral along with profits, minus fees. However, if the runNextFlow function is called after depositing collateral tokens, the protocol becomes stuck in the FLOW.DEPOSIT state after swapping tokens, preventing further operations.

Vulnerability Details

  1. requirements: positionIsClosed == false, beenLong == true and leverage is 1x.

  2. User deposits some collateral tokens.

  3. flow is set as FLOW.DEPOSIT.

  4. as the positionIsClosed is false nextAction.selector is set to NextActionSelector.INCREASE_ACTION.

  5. runNextAction is called by the keeper with metadata of length 1 as the PROTOCOL.DEX.

  6. if (_nextAction.selector == NextActionSelector.INCREASE_ACTION) and if (_isLongOneLeverage(_isLong)) conditions are correct.

  7. _runSwap(metadata, true, prices) function will be called to swap collateral token for index token.

  8. if (metadata.length == 2) this function is false so the else statement will be called.

  9. if (_protocol == PROTOCOL.DEX) and if (flow == FLOW.DEPOSIT) are true so after swapping tokens the _mint() function is called and the necessary shares are allocated to the depositer.

  10. Then the function returns true.

As seen above, the flow is never deleted, causing the protocol to remain stuck in the FLOW.DEPOSIT state, which results in a denial of service (DoS) for the entire contract.

Impact

Users can inadvertently cause a denial of service (DoS) across the entire protocol, or an attacker can intentionally exploit this issue, preventing other users from depositing or withdrawing collateral tokens.

Proof of Concept

Add this function to the PerpetualVault.t.sol and run forge test --mt test_swappingWillBeDoS --via-ir --rpc-url arbitrum -vvv The function will revert with a FlowInProgress() error, causing the protocol to become unresponsive and preventing users from interacting with it.

function test_swappingWillBeDoS() public {
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
vm.deal(alice, 10 ether);
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);
assertEq(uint8(PerpetualVault(vault).flow()), 0);
assertEq(PerpetualVault(vault).positionIsClosed(), false);
(PerpetualVault.NextActionSelector selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
// here the storage variable will be positionIsClose == false, beenLong == true and leverage == 10_000
​
// deposit collateral
depositFixture(alice, 1e10);
​
// nextAction.selector is set to INCREASE_ACTION
(selector, ) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 1);
// set the metaData for paraswap swap.
prices = mockData.getMarketPrices();
paraSwapData = mockData.getParaSwapData(vault);
swapData = new bytes[](1);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
​
// keeper calls the runNextAction function.
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, swapData);
​
// flow is DEPOSIT
uint8 flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 1);
​
address user2 = makeAddr("user2");
​
// this will revert with FlowInProgress() error.
vm.deal(user2, 10 ether);
depositFixture(user2, 1e10);
}

Recommendations

In PerpetualVault::_runSwap call the _finalize function to delete the flow, flowState and swapData

function _runSwap(bytes[] memory metadata, bool isCollateralToIndex, MarketPrices memory prices) internal returns (bool completed) {
....
​
else {
​
....
​
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.