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

Flow not updated after deposit is finalized

0xrststn

Summary

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/PerpetualVault.sol#L350-L409

When a user deposits a long position into a 1x leverage market with a long open position, after the deposit flow is finalized, the flow stays in FLOW.DEPOSIT so the protocol gets stuck. This happens in PerpetualVault::runNextAction

Vulnerability Details

Let`s consider the following scenario:

  • Alice deposits into a market with 1x leverage a long position (first deposit)

  • Bob deposits another long position in the same market after Alice

  • After Bob deposit, the protocol gets stuck in FLOW.DEPOSIT mode

  • Charlie tries to deposit but it reverts

  • Alice tries to withdraw but it reverts
    See the following PoC showing this scenario:
    Add this test to PerpetualVault.t.sol

function test_Deposit_Stuck() external {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
​
// Alice makes the first deposit to the market
payable(alice).transfer(1 ether);
depositFixture(alice, 1e10);
​
MarketPrices memory prices = mockData.getMarketPrices();
​
bytes memory paraSwapData = mockData.getParaSwapData(vault);
bytes[] memory swapData = new bytes[]();
swapData[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
​
address keeper = PerpetualVault(vault).keeper();
​
vm.prank(keeper);
PerpetualVault(vault).run(true, true, prices, swapData);
​
// Bob makes the second deposit to the market
payable(bob).transfer(1 ether);
depositFixture(bob, 1e10);
​
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, swapData);
​
// Flow is still DEPOSIT and it is stuck in it
uint8 flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 1);
​
// Alice tries to withdraw but cannot because the flow is stuck in DEPOSIT
vm.expectRevert();
vm.prank(alice);
PerpetualVault(vault).withdraw(alice, 1);
}
​

Impact

I was hesitant whether to classify it as medium or high. I went for high because even though it can be “fixed” by the keeper, it makes the protocol practically unusable.

After the second deposit, no one can interact with the protocol.
The only fix would be for the keeper to invoke cancelFlow() which will swap the index token back to collateral and transfer it back to Bob in this case and cancel the deposit. By doing this, no more deposits are allowed in this market so all of them will have to be cancelled.

The other possible “fix” would be for the owner to invoke setVaultState() after every deposit to reset the flow.
As stated before, this makes the protocol impractical so I believe it is a high.

Tools Used

Foundry

Recommendations

Invoke _finalize()in runNextAction() after the swap is performed under the conditions : _nextAction.selector == NextActionSelector.INCREASE_ACTION => _isLongOneLeverage(_isLong):

function runNextAction(MarketPrices memory prices, bytes[] memory metadata) external nonReentrant gmxLock {
_onlyKeeper();
Action memory _nextAction = nextAction;
delete nextAction;
if (_nextAction.selector == NextActionSelector.INCREASE_ACTION) {
(bool _isLong) = abi.decode(_nextAction.data, (bool));
​
if (_isLongOneLeverage(_isLong)) {
_runSwap(metadata, true, prices);
++ _finalize(_nextAction.data);
} else {
// swap indexToken that could be generated from the last action into collateralToken
// use only DexSwap
if (
IERC20(indexToken).balanceOf(address(this)) * prices.indexTokenPrice.min >= ONE_USD
) {
(, bytes memory data) = abi.decode(metadata[1], (PROTOCOL, bytes));
_doDexSwap(data, false);
}
(uint256 acceptablePrice) = abi.decode(metadata[0], (uint256));
_createIncreasePosition(_isLong, acceptablePrice, prices);
}
…
…
…
Updates

Lead Judging Commences

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