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

[M-06] Incorrect Precision Scaling in `getPositionInfo()` Can Lead to Miscomputed Values

Summary

The getPositionInfo function in the VaultReader contract performs arithmetic operations on various financial values related to a position, such as collateral, fees, and profit & loss (PnL). However, the calculations do not properly account for precision scaling, leading to potential inaccuracies in computed values. This could affect trading logic, fund calculations, and overall contract behavior.

Vulnerability Details

Function Involved

function getPositionInfo(
bytes32 key,
MarketPrices memory prices
) external view returns (PositionData memory) {
uint256 sizeInTokens = getPositionSizeInUsd(key);
if (sizeInTokens == 0) {
return PositionData({
sizeInUsd: 0,
sizeInTokens: 0,
collateralAmount: 0,
netValue: 0,
pnl: 0,
isLong: true //why do they do this?
});
}
PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
key,
prices,
uint256(0),
address(0),
true
);
uint256 netValue =
positionInfo.position.numbers.collateralAmount * prices.shortTokenPrice.min +
positionInfo.fees.funding.claimableLongTokenAmount * prices.longTokenPrice.min +
positionInfo.fees.funding.claimableShortTokenAmount * prices.shortTokenPrice.min -
positionInfo.fees.borrowing.borrowingFeeUsd -
positionInfo.fees.funding.fundingFeeAmount * prices.shortTokenPrice.min -
positionInfo.fees.positionFeeAmount * prices.shortTokenPrice.min;
if (positionInfo.basePnlUsd >= 0) {
netValue = netValue + uint256(positionInfo.basePnlUsd);
} else {
netValue = netValue - uint256(-positionInfo.basePnlUsd);
}
return PositionData({
sizeInUsd: positionInfo.position.numbers.sizeInUsd,
sizeInTokens: positionInfo.position.numbers.sizeInTokens,
collateralAmount: positionInfo.position.numbers.collateralAmount,
netValue: netValue,
pnl: positionInfo.basePnlUsd,
isLong: positionInfo.position.flags.isLong
});
}

Issue Breakdown

  1. Incorrect Scaling When Multiplying Prices
    The function multiplies token amounts by their respective prices without considering that prices are typically stored with a precision factor (e.g., 1e30 for GMX pricing). This can cause overflows, underflows, or incorrect calculations.

    uint256 netValue =
    positionInfo.position.numbers.collateralAmount * prices.shortTokenPrice.min +
    positionInfo.fees.funding.claimableLongTokenAmount * prices.longTokenPrice.min +
    positionInfo.fees.funding.claimableShortTokenAmount * prices.shortTokenPrice.min -
    positionInfo.fees.borrowing.borrowingFeeUsd -
    positionInfo.fees.funding.fundingFeeAmount * prices.shortTokenPrice.min -
    positionInfo.fees.positionFeeAmount * prices.shortTokenPrice.min;

    Why is this an issue?

    • If prices.shortTokenPrice.min is stored in 1e30 format, then multiplying it directly with an amount stored in 1e18 format will create a huge, incorrect value.

    • If the contract logic expects the result in 1e18 format, then operations relying on netValue may misbehave.

  2. Improper Handling of Negative basePnlUsd

    if (positionInfo.basePnlUsd >= 0) {
    netValue = netValue + uint256(positionInfo.basePnlUsd);
    } else {
    netValue = netValue - uint256(-positionInfo.basePnlUsd);
    }
    • If netValue is already small and basePnlUsd is negative and large, then this could underflow, causing unexpected behavior.

    • There is no safeguard to ensure netValue doesn’t go below 0.

Impact

  • Incorrect Fund Calculation: Due to incorrect precision scaling, calculated netValue can be either too high or too low, affecting portfolio tracking.

  • Potential Underflows: If fees or negative PnL values exceed netValue, arithmetic underflows could lead to unexpected contract states.

  • Broken Trading Logic: The incorrect computation of netValue may cause incorrect liquidation decisions or improper profit/loss tracking.

Tools Used

  • Manual Code Review

Recommendations

1. Apply Precision Scaling to Avoid Miscalculations

Each multiplication involving token amounts and prices should be divided by PRECISION (e.g., 1e30) to keep the values within expected ranges.

uint256 netValue =
positionInfo.position.numbers.collateralAmount * prices.shortTokenPrice.min / PRECISION +
positionInfo.fees.funding.claimableLongTokenAmount * prices.longTokenPrice.min / PRECISION +
positionInfo.fees.funding.claimableShortTokenAmount * prices.shortTokenPrice.min / PRECISION;

2. Separate Fee Calculation for Clarity

Rather than embedding fee subtractions in the same line, explicitly define totalFees:

uint256 totalFees =
positionInfo.fees.borrowing.borrowingFeeUsd +
positionInfo.fees.funding.fundingFeeAmount * prices.shortTokenPrice.min / PRECISION +
positionInfo.fees.positionFeeAmount * prices.shortTokenPrice.min / PRECISION;
netValue = netValue > totalFees ? netValue - totalFees : 0;

This ensures:

  • The total liabilities (fees) are calculated separately.

  • We prevent underflows by ensuring netValue does not go below 0.

3. Handle Negative basePnlUsd Correctly

Instead of:

if (positionInfo.basePnlUsd >= 0) {
netValue = netValue + uint256(positionInfo.basePnlUsd);
} else {
netValue = netValue - uint256(-positionInfo.basePnlUsd);
}

Use:

int256 pnlAdjustment = positionInfo.basePnlUsd;
if (pnlAdjustment > 0) {
netValue = netValue + uint256(pnlAdjustment);
} else {
netValue = netValue > uint256(-pnlAdjustment) ? netValue - uint256(-pnlAdjustment) : 0;
}

This prevents netValue from underflowing if basePnlUsd is negative and too large.

Updates

Lead Judging Commences

n0kto Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.