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

Zero Share Minting Due to Incorrect Price Impact Calculation After Liquidation

Summary

The vulnerability exists in PerpetualVault where users receive zero or near-zero shares when there is pending deposit (flow == DEPOSIT) and right before runNextAction() liquidation is executed by gmx keeper. This occurs due to incorrect price impact calculations in the absence of an active position, leading to excessive deductions from the deposit amount during share minting.

Vulnerability Details

The vulnerability manifests when:

  1. A position is liquidated while a subsequent deposit is pending

  2. The runNextAction() processes the pending deposit (1.2 could be even in one block one-by-one);

The problem code zone is:

// In afterOrderExecution():
if (flow == FLOW.DEPOSIT) {
int256 priceImpact = vaultReader.getPriceImpactInCollateral(
curPositionKey,
orderResultData.sizeDeltaUsd,
prevSizeInTokens,
prices
);
// Problematic calculation when priceImpact is positive
if (priceImpact > 0) {
increased = amount - feeAmount - uint256(priceImpact) - 1;
}
_mint(counter, increased, false, prices);
}

So if priceImpact is positive it is actually deducted from amount to be minted. Let's look at how priceImpact is calculated:

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;
}

So int256 priceImpactInTokens = expectedSizeInTokensDelta.toInt256() - realSizeInTokensDelta.toInt256();
While expectedSizeInTokensDelta it's ovbious that this is sizeDeltaInUsd / prices.indexTokenPrice.min; but realSizeInTokensDelta will be 0 because at the moment of runNextAction() by gamma keeper position has been liquidated. thus realSizeInTokensDelta = 0; //flowData too is 0;
This mean priceImpactInTokens will be equal expectedSizeInTokensDelta;
What leads EVM to drop in this block:

function afterOrderExecution()..
...
if (priceImpact > 0) {
increased = amount - feeAmount - uint256(priceImpact) - 1;
}
...

As we see amount [depositInfo[counter].amount;] minus priceImpact[sizeDeltaInUsd / prices.indexTokenPrice.min;] will leads to mint increased zero or near zero or even revert in some cases.

_mint(counter, increased, false, prices);

Impact

Depositors receive virtually no shares for their deposits or potential revert in some cases;
Funds locked in the contract.

Tools Used

Manual review

Recommendations

Consider to add in runNextAction() additional check whether position has been liquidated or not before proceed deposits.

Updates

Lead Judging Commences

n0kto Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
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.