Liquidity Management

Description

In the deposit() function, since the shares for a new user are calculated based on the balance of collateralToken (collateralToken.balanceOf(address(this))), It is not equal to the vault's actual totalDepositAmount.An attacker can directly transfer additional collateralToken to the vault, causing the calculated shares for the new user to become 0. This disrupts the vault's fund allocation and affects the shares of users.

vulnerability Detail

In the deposit() function, when positionIsClosed is true (i.e., there are no open positions), it directly calls _mint to allocate shares to the new user.

When the position is not open, totalAmountBefore will be calculated by calling _totalAmount(prices).

Therefore, the calculation formula for the user's shares is as follows when positionIsClosed is true:

_shares = amount * totalShares / collateralToken.balanceOf(address(this));

collateralToken.balanceOf(address(this)) is the current balance of collateralToken (e.g., USDC) held by the vault. This balance includes not only the funds deposited by users through the deposit function but also any collateralToken directly transferred to the contract address. If an attacker transfers a large amount of additional collateralToken to the vault before a user deposits,totalAmountBefore will significantly increase, causing the calculated shares for the new user to approach 0.The vault's totalDepositAmount does not increase as expected with the rise of collateralToken.balanceOf(address(this)) to reach maxDepositAmount. Normal user deposits will suffer financial losses.

Since this vulnerability occurs in the _mint() function, when the position is normally opened (i.e.,positionIsClosed is false , beenLong is false), subsequent users who choose to increase their positions will still be affected by this attack. At this point, the attacker could be any previous position holder or even multiple position holders (an attack involving collaboration among multiple position holders would be particularly interesting). Similarly, the financial losses of users increasing their positions will become profits for the position holders, which can then be withdrawn.

When a user call deposit() function to add position，flow flag will set to DEPOSIT，and nextAction.selector will set to INCREASE_ACTION.

then, keeper will call nextAction() to create a MarketIncrease type order in gmxProxy.sol. When the order execute success, afterOrderCancellation() in PerpetualVault.sol will be call by gmxProxy.sol, then go to _mint() function and calculate the shares user can recieve with same logic.

Attack Example:

1. attacker deposit 1000 collateralToken, receive 1000 shares.

2. Attacker Transfers Additional Funds:The attacker transfers 1,000,000 USDC to the vault via collateralToken.transfer(address(vault), 1_000_000).At this point, collateralToken.balanceOf(address(this)) = 1_001_000 USDC.

3. User B Deposits:User B calls deposit(1000) and deposits 1000 USDC.After the deposit, collateralToken.balanceOf(address(this)) = 1_002_000 USDC

4. In _mint: amount = 1000，totalAmountBefore = 1_002_000 - 1000 = 1_001_000, totalShares = 1000.

   thus, _shares = 1000 * 1000 / 1_001_000 ≈ 0.999 ≈ 0 (Solidity integer division rounds down).

5. User B receives _shares of 0.

In an edge case, if the attacker performs a large transfer (e.g., (maxDepositAmount - minDepositAmount)* 1e8 * minDepositAmount + 1), it will cause the shares obtained by any subsequent user depositing an amount to be 0, and the amount deposited by the user will become withdrawable by the attacker.Although the attacker incurs a 5% fee that needs to be transferred to the treasury, if the total loss of users exceeds this amount, the attacker will still profit.

In the normal case，user will also loss funds because shares approve 0. This funds deposited by the user will become withdrawable by the attacker.Although the attacker incurs a 5% fee that needs to be transferred to the treasury, if the total loss of users exceeds this amount, the attacker will still profit.

Amplification Effect in above case. Interestingly, if a large number of small depositors who will obtain 0 shares, their deposit funds will strengthen the attacker’s attack vector (the amount of malicious transfer) because the collateralToken.balanceOf(address(this)) will increase by the small depositors.The reason is small depositors obtain 0 shares eventhought transfer the collateralToken to the vault in the deposit() function. This will require subsequent normal depositors to deposit a higher amount to obtain non-0 shares, and the deposit threshold will gradually increase.This will increases the likelihood of successful attacks and the attacker’s profits.

Impact

1. Users cannot withdraw their funds because their shares are 0. After the attacker withdraws their shares, users will lose their entire deposited amount.

2.Users loss funds because shares are unfairly distributed.

3.The accumulation of funds from small depositors amplifies the attack effect, making the entry cost for normal depositors higher.

Tools Used

foundry

POC

Recommendations

Add Shares Protection Mechanism:

call _cancelFlow() when user'shares be 0 in the _mint() function.

Use Internal Accounting Variable:

Use totalDepositAmount (which tracks the total amount of user deposits) instead of collateralToken.balanceOf(address(this)).Ensure that totalAmountBefore is only updated through deposit and withdraw, unaffected by external transfers.

• For Example, Modify _mint:

  file:PerpetualVault.sol::_mint()

  ​

  function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {

  ...

  if (positionIsClosed) {

  totalAmountBefore = totalDepositAmount - amount;

  }

  ...

  }