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

improper handling the `flow` after calling `_mint` method in the `_runSwap` function

0xjoyboy03

Summary

The vulnerability is related to the improper handling of the flow state in the PerpetualVault contract. After calling the _mint method in the _runSwap() function, the flow remains in the FLOW.DEPOSIT state, preventing further deposits into the contract. This issue arises because the flow state is not finalized after the minting process,

Vulnerability Details

through run or runNextActionfunction call, in many cases, the _runSwapwill be called.

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);
} else if (flow == FLOW.WITHDRAW) {
_handleReturn(outputAmount + swapProgressData.swapped, false, true);
} else {
// in the flow of SIGNAL_CHANGE, if `isCollateralToIndex` is true, it is opening position, or closing position
_updateState(!isCollateralToIndex, isCollateralToIndex);
}
return true;
} else {
_doGmxSwap(data, isCollateralToIndex);
return false;
}
}
}
​

after calling _mintmethod, the flowwon't be finalized and after that no one can deposit into the PerpetualVault contract because the flow == FLOW.DEPOSIT instead of FLOW.NONE . it remains the same till the gmxProxy or owner sets the flow back to none.

here is one scenario:

when the current position is long and the position is open, when the user deposits to the PerpetualVault.sol, the next action is to register the requested position parameters. Then when the keeper calls runNextAction(MarketPrices memory prices, bytes[] memory metadata)

with the metadata.length == 1, the _mintmethod in the _runSwap() will be called and associated shares will be minted for the corresponding user.

in that case the flowis still equal to FLOW.DEPOSIT

here is the POC: (we used setVaultStatesmethod for declaring our scenario simply)

// run: forge test --mt test_unhandling_flow --via-ir --rpc-url arbitrum
function test_unhandling_flow() external {
​
address keeper = PerpetualVault(vault).keeper();
address alice = makeAddr("alice");
payable(alice).transfer(1 ether);
address owner = PerpetualVault(vault).owner();
​
(PerpetualVault.NextActionSelector selector, bytes memory data) = PerpetualVault(vault).nextAction();
​
vm.prank(owner);
PerpetualVault(vault).setVaultState(PerpetualVault.FLOW.NONE, 0, true, bytes32(0), false, false, PerpetualVault.Action(selector, data));
​
depositFixture(alice, 1e10);
​
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory metadata = new bytes[](1);
bytes memory paraSwapData = mockData.getParaSwapData(vault);
metadata[0] = abi.encode(PROTOCOL.DEX, paraSwapData);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, metadata);
​
uint8 flow = uint8(PerpetualVault(vault).flow());
assertEq(flow, 1);
​
address bob = makeAddr("bob");
payable(bob).transfer(1 ether);
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
vm.startPrank(bob);
deal(address(collateralToken), bob, 1e10);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, 1e10);
vm.expectRevert();
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(1e10);
vm.stopPrank();
}

Impact

no one can deposit into the PerpetualVault contract because of not handling the flow state

Tools Used

foundry, certora prover

Recommendations

finalize the flow after the _mint method in the _runSwap:

// 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.