Summary
The PerpetualVault contract contains a critical issue where the deposit function will always fail after a liquidation event due to improper state management in the afterLiquidationExecution function.
Vulnerability Details
-
-
function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
}
if (flow == FLOW.NONE) {
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
} else if (flow == FLOW.DEPOSIT) {
flowData = sizeInTokens;
} else if (flow == FLOW.WITHDRAW) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
}
}
function deposit(uint256 amount) external nonReentrant payable {
_noneFlow();
if (depositPaused == true) {
revert Error.Paused();
}
}
The root of the problem is that when a liquidation occurs, afterLiquidationExecution is called and sets depositPaused = true without any logic ever setting depositPaused back to false after the liquidation is complete. The only way to change depositPaused is via the setDepositPaused function which can only be called by the owner.
As a result, once a liquidation occurs all new deposits will always fail due to depositPaused=true with no automatic mechanism to return the vault to its normal state and the vault is effectively permanently locked to new deposits.
Even after the liquidation is complete and the position is closed (positionIsClosed = true), the vault still cannot accept new deposits because depositPaused remains true.
Impact
POC
Add this to PerpetualVault.t.sol and run it forge test --match-test test_PermanentDepositLockAfterLiquidation --rpc-url arbitrum -vvvv.
function test_PermanentDepositLockAfterLiquidation() external {
address alice = makeAddr("alice");
address bob = makeAddr("bob");
uint256 depositAmount = 1e10;
depositFixture(alice, depositAmount);
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
address keeper = PerpetualVault(vault).keeper();
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
GmxOrderExecuted(true);
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, new bytes[](0));
assertEq(uint8(PerpetualVault(vault).flow()), 0, "Flow should be NONE before liquidation");
assertFalse(PerpetualVault(vault).depositPaused(), "Deposits should not be paused initially");
address gmxProxy = address(PerpetualVault(vault).gmxProxy());
vm.startPrank(gmxProxy);
PerpetualVault(vault).afterLiquidationExecution();
vm.stopPrank();
vm.prank(keeper);
PerpetualVault(vault).runNextAction(prices, new bytes[](0));
assertTrue(PerpetualVault(vault).depositPaused(), "Deposits should be paused after liquidation");
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true) * tx.gasprice;
vm.startPrank(bob);
deal(address(collateralToken), bob, depositAmount);
deal(bob, executionFee);
collateralToken.approve(vault, depositAmount);
vm.expectRevert(Error.Paused.selector);
PerpetualVault(vault).deposit{value: executionFee}(depositAmount);
vm.stopPrank();
vm.warp(block.timestamp + 30 days);
vm.startPrank(bob);
vm.expectRevert(Error.Paused.selector);
PerpetualVault(vault).deposit{value: executionFee}(depositAmount);
vm.stopPrank();
address randomUser = makeAddr("random");
vm.prank(randomUser);
vm.expectRevert("Ownable: caller is not the owner");
PerpetualVault(vault).setDepositPaused(false);
address owner = PerpetualVault(vault).owner();
vm.prank(owner);
PerpetualVault(vault).setDepositPaused(false);
vm.startPrank(bob);
deal(bob, executionFee);
PerpetualVault(vault).deposit{value: executionFee}(depositAmount);
vm.stopPrank();
uint256[] memory bobDeposits = PerpetualVault(vault).getUserDeposits(bob);
assertEq(bobDeposits.length, 1, "Bob should have one deposit after unpausing");
assertFalse(PerpetualVault(vault).depositPaused(), "Deposits should remain unpaused");
}
Tools Used
Recommendations
Implement proper state cleanup and deposit handling during liquidation.
function afterLiquidationExecution() external {
if (msg.sender != address(gmxProxy)) {
revert Error.InvalidCall();
}
depositPaused = true;
uint256 sizeInTokens = vaultReader.getPositionSizeInTokens(curPositionKey);
if (sizeInTokens == 0) {
delete curPositionKey;
}
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(
depositInfo[depositId].owner,
depositInfo[depositId].amount
);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
EnumerableSet.remove(
userDeposits[depositInfo[depositId].owner],
depositId
);
delete depositInfo[depositId];
}
delete flowData;
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
