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

Decimal Precision Mismatch in Price Impact Calculation Leads to Incorrect Share Distribution

Code Areas

Summary

The VaultReader::getPriceImpactInCollateral function fails to account for varying token decimals when converting price impact between tokens, potentially leading to incorrect share calculations. This issue affects the vault's core accounting and could result in significant share misallocations regardless of the decimal combinations of the token pairs used.

Vulnerability Details

The current price impact calculation doesn't account for decimal differences between tokens:

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

The calculation involves:

  • Index token (variable decimals, e.g., ETH: 18, BTC: 8)

  • Collateral token (variable decimals, e.g., USDC: 6)

  • Price feeds: 30 decimals

For example, with ETH/USDC:

priceImpactInTokens (18 decimals)
* indexTokenPrice (30 decimals)
/ shortTokenPrice (30 decimals)
= result (18 decimals) // Should be 6 decimals for USDC

The result maintains the index token's decimals instead of converting to collateral token decimals, leading to incorrect share calculations in PerpetualVault::afterOrderExecution, particularly where increased = amount - feeAmount - uint256(priceImpact) - 1; is calculated and the subsequent _mint() call is made.

Proof of Concept

Example scenarios showing the decimal mismatch:

# Scenario 1: ETH (18) to USDC (6)
priceImpactInTokens = 1 * 10^18 # 1 ETH
indexTokenPrice = 2000 * 10^30 # $2000/ETH
shortTokenPrice = 1 * 10^30 # $1/USDC
result = (1 * 10^18 * 2000 * 10^30) / (1 * 10^30)
= 2000 * 10^18 # Should be 2000 * 10^6 for USDC

Impact Details

  • Incorrect share minting: Users receive wrong number of shares based on their deposits

  • Impact varies based on decimal differences between token pairs

  • Breaks vault's share-to-asset ratio

  • Unfair profit/loss distribution among users

  • Issue becomes more severe with larger decimal differences between tokens

Tools Used

Manual

Recommendations

  1. Track token decimals at contract initialization:

contract VaultReader {
uint256 public immutable collateralDecimals;
uint256 public immutable indexDecimals;
constructor(...) {
collateralDecimals = IERC20Metadata(collateralToken).decimals();
indexDecimals = IERC20Metadata(indexToken).decimals();
}
}
  1. Modify price impact calculation to account for decimal differences:

function getPriceImpactInCollateral(...) {
// ... existing code ...
int256 priceImpactInCollateralTokens = (
priceImpactInTokens
* prices.indexTokenPrice.min.toInt256()
/ prices.shortTokenPrice.min.toInt256()
+ ) / int256(10 ** (indexDecimals - collateralDecimals));
// ... rest of the code ...
}
Updates

Lead Judging Commences

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

invalid_prices_decimals

GMX github documentation: “Prices stored within the Oracle contract represent the price of one unit of the token using a value with 30 decimals of precision. Representing the prices in this way allows for conversions between token amounts and fiat values to be simplified, e.g. to calculate the fiat value of a given number of tokens the calculation would just be: token amount * oracle price, to calculate the token amount for a fiat value it would be: fiat value / oracle price.” Sponsor confirmed the keeper does the same, so price decimals change in function of the token, to be sure the above rule is true. Example for USDC (6 decimals): Prices will have 24 decimals → 1e6 * 1e24 = 1e30. Just a reminder for some submissions: shortToken == collateralTokens, so the decimals is 1e24 for shortToken prices.

Support

FAQs

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