Liquidity Management

Summary

Depositing into the vault is a multi-step operation. First users call deposit and provide funds, second Gamma keeper call runNextAction to create an order in GMX, then GMX keeper execute the order which results in calling afterOrderExecution(). If ADL happens in the time period between order creation and order execution, the calculated priceImpact in afterOrderExecution would be wrongly calculated.

Vulnerability Details

Consider the following scenario:

• We have open position with collateral = 1000 USD, and position size = 3000 USD

• User call deposit(), providing 100 USD.

• Gamma keeper call runNextAction().

• runNextAction internally calls _createIncreasePosition which will set

• Due to highly increased PnL, GMX use the ADL functionality which will trigger GammaProxy::afterOrderExecution():

  else if (msg.sender == address(adlHandler)) {

  uint256 sizeInUsd = dataStore.getUint(keccak256(abi.encode(positionKey, SIZE_IN_USD)));

  if (eventData.uintItems.items[0].value > 0) {

  IERC20(eventData.addressItems.items[0].value).safeTransfer(perpVault, eventData.uintItems.items[0].value);

  }

  if (eventData.uintItems.items[1].value > 0) {

  IERC20(eventData.addressItems.items[1].value).safeTransfer(perpVault, eventData.uintItems.items[1].value);

  }

  if (sizeInUsd == 0) {

  IPerpetualVault(perpVault).afterLiquidationExecution();

  }

• Collateral of the position and size will be lowered and wont be 1000 and 3000 respectively but a smaller values.

• Now GMX keeper executes the initial order for user deposit, which again triggers GmxProxy::afterOrderExecution() which internally calls PerpetualVault::afterOrderExecution() where the shares calculation for user's deposit is done.

  if (orderResultData.orderType == Order.OrderType.MarketIncrease) {

  curPositionKey = positionKey;

  if (flow == FLOW.DEPOSIT) {

  uint256 amount = depositInfo[counter].amount;

  uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;

  uint256 prevSizeInTokens = flowData;

  int256 priceImpact = vaultReader.getPriceImpactInCollateral(curPositionKey, orderResultData.sizeDeltaUsd, prevSizeInTokens, prices);

  uint256 increased;

  if (priceImpact > 0) {

  increased = amount - feeAmount - uint256(priceImpact) - 1;

  } else {

  increased = amount - feeAmount + uint256(-priceImpact) - 1;

  }

  _mint(counter, increased, false, prices);

  nextAction.selector = NextActionSelector.FINALIZE;

  } else {

  _updateState(false, orderResultData.isLong);

  }

  }

• As we have noted above prevSizeInTokens will be based on the value before ADL which is higher than the actual one.

• Taking a look into GetPriceImpactCollateral():

  function getPriceImpactInCollateral(

  bytes32 positionKey,

  uint256 sizeDeltaInUsd,

  uint256 prevSizeInTokens,

  MarketPrices memory prices

  ) external view returns (int256) {

  uint256 expectedSizeInTokensDelta = sizeDeltaInUsd / prices.indexTokenPrice.min;

  uint256 curSizeInTokens = getPositionSizeInTokens(positionKey);

  uint256 realSizeInTokensDelta = curSizeInTokens - prevSizeInTokens;

  int256 priceImpactInTokens = expectedSizeInTokensDelta.toInt256() - realSizeInTokensDelta.toInt256();

  int256 priceImpactInCollateralTokens = priceImpactInTokens * prices.indexTokenPrice.min.toInt256() / prices.shortTokenPrice.min.toInt256();

  return priceImpactInCollateralTokens;

  }

• curSizeInTokens after ADL and user deposit can be either lower than prevSizeInTokens which will revert the callback and Gamma wont be able to account for the user shares or calculated tokens delta would be a lower than actual. This results in incorrect priceImpact calculation which is used to determine the user shares.

Impact

Incorrect calculation of shares for users and even completely failed callback which results in no shares for user and need of manual intervation of owner to setup the next action.

Tools Used

Manual review.

Recommendations

Consider accounting for such case. In GmxProxy::afterOrderExecution when handling ADL scenario, update the flow data (position size in tokens) to match the new one.