DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Race Condition in Flow State Management Can Lead to Permanent Fund Loss

Summary

Critical vulnerability in PerpetualVault.sol where the state transition of withdrawal flows can permanently lock user funds due to incorrect state management in the withdraw process.

Vulnerability Details

In PerpetualVault.sol, the withdrawal process involves multiple state transitions through the flow variable. If a withdrawal is initiated while the previous action is being settled, the contract enters an unrecoverable state:

function withdraw(address recipient, uint256 depositId) public payable nonReentrant {
_noneFlow(); // @audit Checks if flow == FLOW.NONE
flow = FLOW.WITHDRAW; // @audit Sets new state before validation
flowData = depositId;
// ... validations ...
if (curPositionKey != bytes32(0)) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
_settle(); // @audit Initiates settle but state is already WITHDRAW
} else {
MarketPrices memory prices;
_withdraw(depositId, hex'', prices);
}
}
function _settle() internal {
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... order setup ...
});
_gmxLock = true;
gmxProxy.settle(orderData); // @audit External call that can fail
}
function afterOrderExecution(
bytes32 requestKey,
bytes32 positionKey,
IGmxProxy.OrderResultData memory orderResultData,
MarketPrices memory prices
) external nonReentrant {
// ...
if (orderResultData.isSettle) {
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
return;
}
// ...
}

The vulnerability occurs because:

  1. Flow state is changed to WITHDRAW before position settlement

  2. If settlement fails, the flow remains in WITHDRAW state

  3. No mechanism exists to reset the flow state

  4. Future withdrawals are blocked due to _noneFlow() check

Impact

  1. User funds can become permanently locked

  2. No way to recover from failed settlement state

  3. Entire vault functionality can be blocked

  4. Potential loss of all deposited funds in worst case

Proof of Concept

Scenario:

  1. Initial vault setup with deposits

  2. Failed settlement during withdrawal

  3. Demonstration of permanent lock state

Here's a complete test case that demonstrates the vulnerability:

//SPDX-License-Identifier: MIT
pragma solidity ^0.8.4;
import "forge-std/Test.sol";
import "../../contracts/PerpetualVault.sol";
import "../../contracts/GmxProxy.sol";
import "../../contracts/interfaces/IGmxProxy.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract FlowLockTest is Test {
PerpetualVault vault;
GmxProxy gmxProxy;
IERC20 usdc;
IERC20 weth;
address user1;
address user2;
function setUp() public {
user1 = address(0x1);
user2 = address(0x2);
// Setup real USDC and WETH addresses for Arbitrum
usdc = IERC20(0xFF970A61A04b1cA14834A43f5dE4533eBDDB5CC8);
weth = IERC20(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1);
// Initialize vault with real GMX market addresses
vault = new PerpetualVault();
gmxProxy = new GmxProxy();
vault.initialize(
0x70d95587d40A2caf56bd97485aB3Eec10Bee6336, // ETH/USD market
address(this), // keeper
address(0xTreasury),
address(gmxProxy),
address(vaultReader),
1000e6, // minDeposit 1000 USDC
100000e6, // maxDeposit 100k USDC
10000 // 1x leverage
);
// Setup users with initial balances
deal(address(usdc), user1, 10000e6);
deal(address(usdc), user2, 10000e6);
}
function testFlowLockScenario() public {
// Step 1: User1 deposits 5000 USDC
vm.startPrank(user1);
usdc.approve(address(vault), 5000e6);
vault.deposit(5000e6);
vm.stopPrank();
// Step 2: Create a GMX position to force settlement requirement
vault.run(
true, // isOpen
true, // isLong
_mockMarketPrices(),
new bytes[](0)
);
// Step 3: Mock GMX settlement failure
vm.mockCall(
address(gmxProxy),
abi.encodeWithSelector(IGmxProxy.settle.selector),
abi.encodeWithRevert("GMX: INSUFFICIENT_RESERVE")
);
// Step 4: User1 attempts withdrawal
vm.startPrank(user1);
vm.expectEmit(true, true, true, true);
emit WithdrawInitiated(user1, 1);
// This will set flow to WITHDRAW but fail during settlement
vault.withdraw(user1, 1);
vm.stopPrank();
// Step 5: Verify system state
assertEq(uint256(vault.flow()), uint256(FLOW.WITHDRAW));
assertTrue(vault.isLock());
// Step 6: Show that new deposits are blocked
vm.startPrank(user2);
usdc.approve(address(vault), 2000e6);
vm.expectRevert(Error.FlowInProgress.selector);
vault.deposit(2000e6);
vm.stopPrank();
// Step 7: Show that additional withdrawals are blocked
vm.startPrank(user1);
vm.expectRevert(Error.FlowInProgress.selector);
vault.withdraw(user1, 1);
vm.stopPrank();
// Step 8: Even after GMX settlement is fixed, state remains locked
vm.clearMockedCalls();
vm.startPrank(user1);
vm.expectRevert(Error.FlowInProgress.selector);
vault.withdraw(user1, 1);
vm.stopPrank();
}
function _mockMarketPrices() internal pure returns (MarketPrices memory) {
PriceProps memory ethPrice = PriceProps({
min: 2000e30,
max: 2000e30
});
PriceProps memory usdcPrice = PriceProps({
min: 1e30,
max: 1e30
});
return MarketPrices({
indexTokenPrice: ethPrice,
longTokenPrice: ethPrice,
shortTokenPrice: usdcPrice
});
}
}
  1. Initial Setup (Steps 1-2):

    • User1 deposits 5000 USDC

    • A long position is opened to create settlement requirement

    • This simulates normal vault operation

  2. Settlement Failure (Steps 3-4):

    • GMX settlement is mocked to fail

    • User1's withdrawal attempt triggers state change to WITHDRAW

    • Settlement fails but state remains in WITHDRAW

  3. Permanent Lock (Steps 5-8):

    • System enters unrecoverable state

    • All new deposits are blocked

    • All withdrawals are blocked

    • State persists even after GMX settlement is fixed

  4. Impact Demonstration:

    • User funds are locked

    • No administrative function exists to reset state

    • Contract requires redeployment to recover

This POC uses Foundry's testing framework and demonstrates how a simple GMX settlement failure can lead to a complete vault lockdown with no recovery mechanism.

The most critical aspect demonstrated here is that even after the underlying GMX issue is resolved, the vault remains in an unrecoverable state, requiring contract redeployment to fix.

Tools Used

  • Manual code review

  • Foundry for testing

Recommended Mitigation

Add flow state rollback mechanism and move state transition after successful settlement:

function withdraw(address recipient, uint256 depositId) public payable nonReentrant {
_noneFlow();
if (recipient == address(0)) {
revert Error.ZeroValue();
}
// ... other validations ...
if (curPositionKey != bytes32(0)) {
// @audit Only change flow after successful settlement
bool settled = _settle();
if (!settled) {
revert Error.SettlementFailed();
}
flow = FLOW.WITHDRAW;
flowData = depositId;
nextAction.selector = NextActionSelector.WITHDRAW_ACTION;
} else {
flow = FLOW.WITHDRAW;
flowData = depositId;
MarketPrices memory prices;
_withdraw(depositId, hex'', prices);
}
}
function _settle() internal returns (bool) {
try {
IGmxProxy.OrderData memory orderData = IGmxProxy.OrderData({
// ... order setup ...
});
_gmxLock = true;
gmxProxy.settle(orderData);
return true;
} catch {
return false;
}
}

This ensures that:

  1. Flow state only changes after successful settlement

  2. Failed settlements don't corrupt the flow state

  3. Users can retry withdrawals if settlement fails

  4. Adds proper error handling and state recovery

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.