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

Calculating price impact collateral is incorrect when calculating users' increase from deposit

vinica_boy

Summary

When calculating the price impact from an increase position, we first calculate what is the expected amount without price impact and what is the actual amount. Based on the difference we get the price impact. The problem in Gamma implementation is that for both short and long positions, we use min price of index token to get the size delta in token when we should be using max price for long positions and min price for short positions.

Vulnerability Details

Taking a look into PositionUtils::getExecutionPriceForIncrease() in GMX code, we first calculate baseSizeDeltaInTokens which is the amount without price impact (this is semantically the same as expected delta in size tokens in Gamma).

uint256 baseSizeDeltaInTokens;
​
if (params.position.isLong()) {
// round the number of tokens for long positions down
baseSizeDeltaInTokens = params.order.sizeDeltaUsd() / indexTokenPrice.max;
} else {
// round the number of tokens for short positions up
baseSizeDeltaInTokens = Calc.roundUpDivision(params.order.sizeDeltaUsd(), indexTokenPrice.min);
}

We see that there is different calculation of the amount of tokens based on if the position is short or long, while in Gamma (VaultReader::getPriceImpactInCollateral()) we always use min price:

function getPriceImpactInCollateral(
bytes32 positionKey,
uint256 sizeDeltaInUsd,
uint256 prevSizeInTokens,
MarketPrices memory prices
) external view returns (int256) {
// @audit min/max depending on short/long
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;
}

Impact

Incorrect price impact calculations leading to unfair shares allocation since users' deposits are adjusted based on the calculated price impact when minting shares.

Tools Used

Manual review.

Recommendations

Consider checking if position is long or short and use the correct price to match the actual calculation in GMX.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

Informational or Gas

Please read the CodeHawks documentation to know which submissions are valid. If you disagree, provide a coded PoC and explain the real likelihood and the detailed impact on the mainnet without any supposition (if, it could, etc) to prove your point.

Appeal created

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

finding_priceImpact_not_using_indexTokenPrice.max_for_long_position

Likelihood: High, every long position. Impact: Informational/Very Low. Long positions should use the ask price to calculate `expectedSizeInTokensDelta`. This will lead to a slight miscalculation of the shares. However, since the same calculation is applied to every user, and all users are in the same position (short or long), it won’t have a real impact. Moreover, both prices always have a very small difference.

Support

FAQs

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

Give us feedback!