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

`PerpetualVault::deposit` flow never reset

izuman

Summary

After calling the PerpetualVault::deposit and initiating the deposit flow. The vault contract never completes the flow and the flow is stuck where flow == FLOW.DEPOSIT

Vulnerability Details

The problem is that after deposit is made the shares are minted to the user after the keeper performs the runNextAction call, but the flow is never reset. This will lock the contract in the FLOW.DEPOSIT and actions will be paused until the emergency PerpetualVault::setVaultState function is called and manually resets the state. However, this should not be done during regular operation and after every single deposit.

The issue occurs when the vault settings are 1x leverage and long. Thus, all deposits will be routed to the Paraswap DEX.

PoC

Paste the following PoC into the PerpetualVault.t.sol file and run with forge test --mt test_deposit_flow_not_reset --rpc-url https://arb1.arbitrum.io/rpc -vvv

/// @notice PoC for stuck flow during deposit
function test_deposit_flow_not_reset() external {
//forge test --mt test_deposit_flow_not_reset --rpc-url https://arb1.arbitrum.io/rpc -vvv
/*
Vault is 1x Long vault so the deposits will trigger a DEX swap on Paraswap by the Keeper
*/
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
//This will deposit 1e10 USDC into the vault
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);
//For the first deposit the flow will be correctly reset
assertEq(uint8(PerpetualVault(vault).flow()), 0);
assertEq(PerpetualVault(vault).positionIsClosed(), false);
(PerpetualVault.NextActionSelector selector,) = PerpetualVault(vault).nextAction();
assertEq(uint8(selector), 0);
// assertEq(uint8(PerpetualVault(vault).isNextAction()), 0);
​
//Next a second deposit is made and will follow a different path now that a position is open
//There will be an execution cost so we will give alice more than enough Ethereum
deal(alice, 1 ether);
//deposit again
depositFixture(alice, 1e10);
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
vm.prank(keeper);
//this should revert since the flow == FLOW.DEPOSIT , This is expected
vm.expectRevert(bytes4(keccak256("FlowInProgress()")));
PerpetualVault(vault).run(true, true, prices, swapData);
​
//we need to call `runNextAction`
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, swapData);
​
//however the flow will not be reset even though the deposit flow has been completed
assertEq(uint8(PerpetualVault(vault).flow()), 1); // FLOW.DEPOSIT == 1
​
//get the deposit info of alice to verify shares have been minted for the second deposit thus `2`
(, uint256 shares,,,,) = PerpetualVault(vault).depositInfo(2);
console.log("Alice's share amount for second deposit is: ", shares);
​
/*
ISSUE:
The shares were succesfully minted for the second deposit however, the flow was not reset.
*/
//If we try to deposit again it will be revert with the "FlowInProgress()" error
uint256 finalDepositAmount = 1e10;
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
vm.startPrank(alice);
deal(address(collateralToken), alice, finalDepositAmount);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, finalDepositAmount);
vm.expectRevert(bytes4(keccak256("FlowInProgress()")));
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(finalDepositAmount);
vm.stopPrank();
}
​

Impact

Contract will be in a frozen state until the admin manually resets the flow.

Tools Used

Manual Review and Foundry

Recommendations

Reset the flow inside PerpetualVault::_mint so that flow is correctly reset.

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!