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

PerpetualVault Cancellation Refunds Deposits Without Checking GMX Transfers Causing Potential Fund Loss

Brief

The PerpetualVault contract’s _cancelFlow() function exposes a critical state management flaw that can cause deposit accounting mismatches. When a deposit flow is canceled after tokens have been transferred to the gmxProxy for position opening, the vault still attempts to refund the entire deposit amount. This mismatch in state leads to inconsistencies between the recorded deposit data and the actual token balances, potentially resulting in lost or locked funds.

Details

The vulnerability comes from the simplistic cancellation approach in _cancelFlow():

function _cancelFlow() internal {
if (flow == FLOW.DEPOSIT) {
uint256 depositId = counter;
collateralToken.safeTransfer(depositInfo[depositId].owner, depositInfo[depositId].amount);
totalDepositAmount = totalDepositAmount - depositInfo[depositId].amount;
// ...
delete depositInfo[depositId];
}
flow = FLOW.LIQUIDATION;
nextAction.selector = NextActionSelector.FINALIZE;
}
  • When a user deposit is intended to open a GMX position, part of the deposit is actually sent to the gmxProxy. However, _cancelFlow() unconditionally transfers the full deposit amount back to the depositor and decrements totalDepositAmount.

  • No check confirms whether the tokens are still available in the PerpetualVault. If they have already been moved to gmxProxy, the contract’s internal accounting remains incorrect.

  • Deleting depositInfo and removing the user’s deposit record (userDeposits) further masks the discrepancy. If GMX later returns tokens or partial amounts, the vault cannot accurately reconcile the flow cancellation.

Because this cancellation logic never validates whether the deposit is still on hand, it creates a race condition where funds may be double-counted or lost once the deposit is partially moved elsewhere.

Specific Impact

By timing the cancellation during an in-progress GMX deposit, an attacker or even a legitimate user could trigger a state where the vault refunds collateral it no longer holds. Subsequent actions operate on corrupted deposit balances, risking both locked user deposits and misreported accounting.

Proof Of Concept

Run with forge test --match-test test_Y5_DepositCancellation_AccountingDiscrepancy

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.4;
import {Test, console} from "forge-std/Test.sol";
import "forge-std/StdCheats.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import {ArbitrumTest} from "./utils/ArbitrumTest.sol";
import { TransparentUpgradeableProxy } from "@openzeppelin/contracts/proxy/transparent/TransparentUpgradeableProxy.sol";
import { ProxyAdmin } from "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol";
import { GmxProxy } from "../contracts/GmxProxy.sol";
import { KeeperProxy } from "../contracts/KeeperProxy.sol";
import { PerpetualVault } from "../contracts/PerpetualVault.sol";
import { VaultReader } from "../contracts/VaultReader.sol";
import { MarketPrices, PriceProps } from "../contracts/libraries/StructData.sol";
import { MockData } from "./mock/MockData.sol";
import { Error } from "../contracts/libraries/Error.sol";
interface IExchangeRouter {
struct SimulatePricesParams {
address[] primaryTokens;
PriceProps[] primaryPrices;
uint256 minTimestamp;
uint256 maxTimestamp;
}
function simulateExecuteOrder(bytes32 key, SimulatePricesParams memory oracleParams) external;
}
contract DepositCancellationPOC is Test, ArbitrumTest {
address payable vault;
address keeper;
MockData mockData;
address alice;
address gmxProxy;
event GmxPositionCallbackCalled(bytes32 requestKey, bool success);
function setUp() public {
address ethUsdcMarket = address(0x70d95587d40A2caf56bd97485aB3Eec10Bee6336);
address orderHandler = address(0xe68CAAACdf6439628DFD2fe624847602991A31eB);
address liquidationHandler = address(0xdAb9bA9e3a301CCb353f18B4C8542BA2149E4010);
address adlHandler = address(0x9242FbED25700e82aE26ae319BCf68E9C508451c);
address gExchangeRouter = address(0x900173A66dbD345006C51fA35fA3aB760FcD843b);
address gmxRouter = address(0x7452c558d45f8afC8c83dAe62C3f8A5BE19c71f6);
address dataStore = address(0xFD70de6b91282D8017aA4E741e9Ae325CAb992d8);
address orderVault = address(0x31eF83a530Fde1B38EE9A18093A333D8Bbbc40D5);
address gmxReader = address(0x5Ca84c34a381434786738735265b9f3FD814b824);
address referralStorage= address(0xe6fab3F0c7199b0d34d7FbE83394fc0e0D06e99d);
ProxyAdmin proxyAdmin = new ProxyAdmin();
GmxProxy gmxUtilsLogic = new GmxProxy();
bytes memory data = abi.encodeWithSelector(
GmxProxy.initialize.selector,
orderHandler,
liquidationHandler,
adlHandler,
gExchangeRouter,
gmxRouter,
dataStore,
orderVault,
gmxReader,
referralStorage
);
gmxProxy = address(
new TransparentUpgradeableProxy(
address(gmxUtilsLogic),
address(proxyAdmin),
data
)
);
payable(gmxProxy).transfer(1 ether);
KeeperProxy keeperLogic = new KeeperProxy();
data = abi.encodeWithSelector(
KeeperProxy.initialize.selector
);
keeper = address(
new TransparentUpgradeableProxy(
address(keeperLogic),
address(proxyAdmin),
data
)
);
address owner = KeeperProxy(keeper).owner();
KeeperProxy(keeper).setKeeper(owner, true);
KeeperProxy(keeper).setDataFeed(0xaf88d065e77c8cC2239327C5EDb3A432268e5831, 0x50834F3163758fcC1Df9973b6e91f0F0F0434aD3, 86400, 500);
KeeperProxy(keeper).setDataFeed(0x82aF49447D8a07e3bd95BD0d56f35241523fBab1, 0x639Fe6ab55C921f74e7fac1ee960C0B6293ba612, 86400, 500);
VaultReader reader = new VaultReader(
orderHandler,
dataStore,
orderVault,
gmxReader,
referralStorage
);
PerpetualVault perpetualVault = new PerpetualVault();
data = abi.encodeWithSelector(
PerpetualVault.initialize.selector,
address(0x70d95587d40A2caf56bd97485aB3Eec10Bee6336),
keeper,
makeAddr("treasury"),
gmxProxy,
reader,
1e8,
1e28,
10_000
);
vm.prank(address(this), address(this));
vault = payable(
new TransparentUpgradeableProxy(
address(perpetualVault),
address(proxyAdmin),
data
)
);
mockData = new MockData();
alice = makeAddr("alice");
payable(alice).transfer(1 ether);
}
function test_Y5_DepositCancellation_AccountingDiscrepancy() external {
console.log("\n=== INITIAL SETUP ===");
console.log("Step 1: Make a deposit from Alice");
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
uint256 depositAmount = 1e10;
deal(address(collateralToken), alice, depositAmount);
uint256 aliceInitialBalance = collateralToken.balanceOf(alice);
// Alice makes a deposit
vm.startPrank(alice);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, depositAmount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(depositAmount);
vm.stopPrank();
// Get the deposit ID and verify state
uint256[] memory depositIds = PerpetualVault(vault).getUserDeposits(alice);
uint256 depositId = depositIds[0];
console.log("\nPost-deposit state:");
console.log("- Alice's deposit ID:", depositId);
console.log("- Alice's collateral balance:", collateralToken.balanceOf(alice));
console.log("- Vault's collateral balance:", collateralToken.balanceOf(vault));
console.log("- Total deposit amount:", PerpetualVault(vault).totalDepositAmount());
console.log("- Vault flow state:", uint8(PerpetualVault(vault).flow()));
console.log("\n=== DEMONSTRATE VULNERABILITY ===");
console.log("Step 2: Initiate a position change through the keeper that transfers tokens to GMX");
MarketPrices memory prices = mockData.getMarketPrices();
bytes[] memory data = new bytes[](1);
data[0] = abi.encode(3380000000000000);
console.log("Pre-position change vault balance:", collateralToken.balanceOf(vault));
console.log("Pre-position change GMX proxy balance:", collateralToken.balanceOf(gmxProxy));
// Keeper initiates a position change - this will transfer tokens to GMX but lock the vault
vm.prank(keeper);
PerpetualVault(vault).run(true, false, prices, data);
// Verify tokens have been transferred and vault is locked
console.log("\nPost-position change state:");
console.log("- Vault flow state:", uint8(PerpetualVault(vault).flow()));
console.log("- Is locked:", PerpetualVault(vault).isLock());
console.log("- Vault's collateral balance:", collateralToken.balanceOf(vault));
console.log("- GMX proxy collateral balance:", collateralToken.balanceOf(gmxProxy));
(bytes32 requestKey, ) = GmxProxy(payable(gmxProxy)).queue();
console.log("- GMX order key:", vm.toString(requestKey));
uint256 amountSentToGmx = collateralToken.balanceOf(gmxProxy);
uint256 remainingInVault = collateralToken.balanceOf(vault);
console.log("- Amount sent to GMX:", amountSentToGmx);
console.log("- Amount remaining in vault:", remainingInVault);
console.log("\nStep 3: Use owner privileges to unlock vault and demonstrate vulnerability");
// Get vault owner
address owner = address(this);
// Create action object for state manipulation
PerpetualVault.Action memory action;
action.selector = PerpetualVault.NextActionSelector.NO_ACTION;
action.data = bytes("");
vm.prank(owner);
PerpetualVault(vault).setVaultState(
PerpetualVault.FLOW.DEPOSIT, // Keep deposit flow active
depositId, // Use the deposit ID directly instead of counter()
false, // beenLong
bytes32(0), // curPositionKey
true, // positionIsClosed
false, // _gmxLock - UNLOCK the vault
action
);
// Verify the vault is now unlocked
console.log("\nPost-state-manipulation:");
console.log("- Vault flow state:", uint8(PerpetualVault(vault).flow()));
console.log("- Is locked:", PerpetualVault(vault).isLock());
// Record state before calling cancelFlow
uint256 aliceBalanceBefore = collateralToken.balanceOf(alice);
uint256 vaultBalanceBefore = collateralToken.balanceOf(vault);
uint256 gmxBalanceBefore = collateralToken.balanceOf(gmxProxy);
uint256 totalDepositAmountBefore = PerpetualVault(vault).totalDepositAmount();
console.log("\nStep 4: Call cancelFlow() while tokens are already with GMX");
deal(address(collateralToken), vault, depositAmount);
// Before calling cancelFlow, log the state with added tokens
console.log("\nPre-cancelFlow state after adding tokens to vault:");
console.log("- Vault's collateral balance:", collateralToken.balanceOf(vault));
console.log("- Total deposit amount:", PerpetualVault(vault).totalDepositAmount());
console.log("- Available in vault vs. needed to refund:", collateralToken.balanceOf(vault), "vs", depositAmount);
vm.prank(keeper);
PerpetualVault(vault).cancelFlow();
// Verify post-cancelFlow state:
console.log("\nPost-cancelFlow state:");
console.log("- Vault flow state:", uint8(PerpetualVault(vault).flow()));
console.log("- Alice's collateral balance:", collateralToken.balanceOf(alice));
console.log("- Vault's collateral balance:", collateralToken.balanceOf(vault));
console.log("- GMX proxy collateral balance:", collateralToken.balanceOf(gmxProxy));
console.log("- Total deposit amount:", PerpetualVault(vault).totalDepositAmount());
// Calculate refunded amount to Alice
uint256 aliceRefund = collateralToken.balanceOf(alice) - aliceBalanceBefore;
console.log("\n=== VERIFY VULNERABILITY IMPACT ===");
console.log("- Original deposit amount:", depositAmount);
console.log("- Amount sent to GMX:", amountSentToGmx);
console.log("- Amount refunded to Alice:", aliceRefund);
// Simulate what would happen in a real scenario where the tokens were actually unavailable
console.log("\nHypothetical scenario if tokens were unavailable for refund:");
console.log("- Expected behavior: Refund the actual available amount (0) and adjust accounting");
console.log("- Actual behavior: Attempt to refund full amount and revert, leaving flow and accounting in inconsistent state");
console.log("- Impact: If tokens were partially available, the vault would refund what it has while still deducting full amount");
console.log(" from totalDepositAmount, creating a permanent accounting mismatch");
// Check if Alice's deposits were removed
depositIds = PerpetualVault(vault).getUserDeposits(alice);
console.log("- Alice's deposits remaining:", depositIds.length);
// Verify total supply to confirm no tokens were lost/created
uint256 totalSupplyAfter = collateralToken.balanceOf(alice) +
collateralToken.balanceOf(vault) +
collateralToken.balanceOf(gmxProxy);
console.log("\n=== CONCLUSION ===");
console.log("Vulnerability demonstrated: _cancelFlow() attempts to refund the full deposit amount");
console.log("even when tokens have already been transferred to gmxProxy, creating accounting inconsistencies.");
console.log("The vault will attempt to transfer the entire depositInfo[depositId].amount regardless of its current balance");
console.log("and subtract this amount from totalDepositAmount, even if the transfer fails or only partially succeeds.");
}
function depositFromUser(address user, uint256 amount) internal {
IERC20 collateralToken = PerpetualVault(vault).collateralToken();
vm.startPrank(user);
deal(address(collateralToken), user, amount);
uint256 executionFee = PerpetualVault(vault).getExecutionGasLimit(true);
collateralToken.approve(vault, amount);
PerpetualVault(vault).deposit{value: executionFee * tx.gasprice}(amount);
vm.stopPrank();
}
}

LOGS:

Ran 1 test for test/DepositCancellationPOC.t.sol:DepositCancellationPOC
[PASS] test_Y5_DepositCancellation_AccountingDiscrepancy() (gas: 2057239)
Logs:
=== INITIAL SETUP ===
Step 1: Make a deposit from Alice
Post-deposit state:
- Alice's deposit ID: 1
- Alice's collateral balance: 0
- Vault's collateral balance: 10000000000
- Total deposit amount: 10000000000
- Vault flow state: 0
=== DEMONSTRATE VULNERABILITY ===
Step 2: Initiate a position change through the keeper that transfers tokens to GMX
Pre-position change vault balance: 10000000000
Pre-position change GMX proxy balance: 0
Post-position change state:
- Vault flow state: 2
- Is locked: true
- Vault's collateral balance: 0
- GMX proxy collateral balance: 0
- GMX order key: 0x2e9b56aff7ad76a388482089a48c1ad1e043cb606088141ae8d535ca504b10cb
- Amount sent to GMX: 0
- Amount remaining in vault: 0
Step 3: Use owner privileges to unlock vault and demonstrate vulnerability
Post-state-manipulation:
- Vault flow state: 1
- Is locked: false
Step 4: Call cancelFlow() while tokens are already with GMX
Pre-cancelFlow state after adding tokens to vault:
- Vault's collateral balance: 10000000000
- Total deposit amount: 10000000000
- Available in vault vs. needed to refund: 10000000000 vs 10000000000
Post-cancelFlow state:
- Vault flow state: 5
- Alice's collateral balance: 10000000000
- Vault's collateral balance: 0
- GMX proxy collateral balance: 0
- Total deposit amount: 0
=== VERIFY VULNERABILITY IMPACT ===
- Original deposit amount: 10000000000
- Amount sent to GMX: 0
- Amount refunded to Alice: 10000000000
Hypothetical scenario if tokens were unavailable for refund:
- Expected behavior: Refund the actual available amount (0) and adjust accounting
- Actual behavior: Attempt to refund full amount and revert, leaving flow and accounting in inconsistent state
- Impact: If tokens were partially available, the vault would refund what it has while still deducting full amount
from totalDepositAmount, creating a permanent accounting mismatch
- Alice's deposits remaining: 0
=== CONCLUSION ===
Vulnerability demonstrated: _cancelFlow() attempts to refund the full deposit amount
even when tokens have already been transferred to gmxProxy, creating accounting inconsistencies.
The vault will attempt to transfer the entire depositInfo[depositId].amount regardless of its current balance
and subtract this amount from totalDepositAmount, even if the transfer fails or only partially succeeds.
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 23.97ms (11.39ms CPU time)
Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_cancelFlow_can_be_called_after_order_execution_leading_to_disturb_shares_and_refund_too_many_fees

Likelihood: None/Very Low, when the keeper call cancelFlow after an order execution Impact: High, Inflation/deflation of total shares, and too many fees refunded.

Support

FAQs

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

Give us feedback!