Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Incorrect Price Selection in Price Impact Calculation Leads to Share Value Exaggeration

krisrenzo

Summary

The VaultReader.getPriceImpactInCollateral() function consistently uses the minimum index token price when calculating price impact, rather than using the maximum price which would provide a more conservative estimate. This results in a smaller price impact value being calculated, leading to users receiving more shares than they should when depositing into the vault.

Vulnerability Details

When users deposit into the vault and a GMX position is opened, the number of shares minted is calculated based on the deposit amount minus fees and price impact. The price impact calculation in VaultReader.getPriceImpactInCollateral() is critical to this process:

int256 priceImpactInCollateral(
bytes32 positionKey,
uint256 sizeDeltaInUsd,
uint256 prevSizeInTokens,
MarketPrices memory prices
) {
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;
}

The function always uses prices.indexTokenPrice.min when it should use prices.indexTokenPrice.max to calculate a more conservative price impact value. This affects the share calculation in PerpetualVault.sol:

uint256 feeAmount = vaultReader.getPositionFeeUsd(market, orderResultData.sizeDeltaUsd, false) / prices.shortTokenPrice.min;
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);

This is what _mint function looks like

function _mint(uint256 depositId, uint256 amount, bool refundFee, MarketPrices memory prices) internal {
uint256 _shares;
// ...
if (totalAmountBefore == 0) totalAmountBefore = 1;
_shares = amount * totalShares / totalAmountBefore;
//...
​
depositInfo[depositId].shares = _shares;
totalShares = totalShares + _shares;
// ...
}

Proof of Concept

Consider a scenario where:

  • Index token price range: min = $1000, max = $1020

  • Size delta in USD: 10,000

  • Previous size in tokens: 9.8

  • Current size in tokens: 10

Using minimum price:

expectedSizeInTokensDelta = 10,000 / 1000 = 10
realSizeInTokensDelta = 10 - 9.8 = 0.2
priceImpactInTokens = 10 - 0.2 = 9.8
priceImpactInCollateralTokens = 9.8 * 1000 = 9,800

Using maximum price (correct):

expectedSizeInTokensDelta = 10,000 / 1020 = 9.803
realSizeInTokensDelta = 10 - 9.8 = 0.2
priceImpactInTokens = 9.803 - 0.2 = 9.603
priceImpactInCollateralTokens = 9.603 * 1020 = 9,795.06

The difference results in users receiving more shares than they should.

Impact Details

The impact is that users receive more shares than they should when depositing into the vault, which:

  1. Dilutes the value of existing shares

  2. Creates an unfair advantage for depositing users

  3. Could lead to economic losses for the protocol if exploited at scale

The severity is medium because while it does lead to economic damage, the impact is limited by:

  • The price spread between min and max prices

  • The amount of deposits that can be made

  • The maximum deposit caps in place

  • And a high likelihood since is happens everytime getPriceImpactInCollateral is called

Recommendations

Modify the getPriceImpactInCollateral function to use the maximum index token price:

function getPriceImpactInCollateral(
bytes32 positionKey,
uint256 sizeDeltaInUsd,
uint256 prevSizeInTokens,
MarketPrices memory prices
) external view returns (int256) {
uint256 expectedSizeInTokensDelta = sizeDeltaInUsd / prices.indexTokenPrice.max; // Use max price
uint256 curSizeInTokens = getPositionSizeInTokens(positionKey);
uint256 realSizeInTokensDelta = curSizeInTokens - prevSizeInTokens;
int256 priceImpactInTokens = expectedSizeInTokensDelta.toInt256() - realSizeInTokensDelta.toInt256();
int256 priceImpactInCollateralTokens = priceImpactInTokens * prices.indexTokenPrice.max.toInt256() / prices.shortTokenPrice.min.toInt256(); // Use max price
return priceImpactInCollateralTokens;
}

This ensures a more conservative price impact calculation that properly protects the protocol and existing share holders.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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