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

Incorrect calculation of `realizedPnlUsd` in `VaultReader::willPositionCollateralBeInsufficient` leads to an overestimation of remaining collateral and a risk of unexpected position liquidation

Summary

A vulnerability exists in the VaultReader::willPositionCollateralBeInsufficient function that can lead to an overestimation of remaining collateral when decreasing a long position. The issue arises due to an incorrect method of calculating realized profit and loss (PnL), which does not account for rounding differences in how the GMX protocol computes realized PnL. This can result in positions being closed prematurely, exposing users to unnecessary liquidations.

Vulnerability Details

The vulnerability originates in the following portion of the VaultReader::willPositionCollateralBeInsufficient function:

PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
positionKey,
prices,
uint256(0),
address(0),
true
);
int256 realizedPnlUsd;
if (positionInfo.basePnlUsd > 0) {
realizedPnlUsd = (uint256(positionInfo.basePnlUsd) * sizeDeltaUsd / positionInfo.position.numbers.sizeInUsd).toInt256();
} else {
realizedPnlUsd = -(uint256(-positionInfo.basePnlUsd) * uint256(sizeDeltaUsd) / uint256(positionInfo.position.numbers.sizeInUsd)).toInt256();
}

The issue arises because the function gmxReader.getPositionInfo is called with the last parameter set to true, meaning basePnlUsd is calculated for the entire position rather than for the reduced portion (sizeDeltaUsd). The function then computes the realized PnL for sizeDeltaUsd by scaling basePnlUsd proportionally. The realized PnL (if negative) is then subtracted from the collateral.

However, this approach does not align with how the GMX protocol calculates the realized PnL and decreases collateral amounts. Namely, in DecreasePositionCollateralUtils::processCollateral (https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/DecreasePositionCollateralUtils.sol#L54) it is stated that "the basePnlUsd is the pnl to be realized, and is calculated as: totalPositionPnl * sizeDeltaInTokens / position.sizeInTokens()", i.e sizeDeltaUsd is converted to tokens first, and then the totalPositionPnl is scaled:

(values.basePnlUsd, values.uncappedBasePnlUsd, values.sizeDeltaInTokens) = PositionUtils.getPositionPnlUsd(
params.contracts.dataStore,
params.market,
cache.prices,
params.position,
params.order.sizeDeltaUsd()
);

where the calculation of values.basePnlUsd can be found in PositionUtils::getPositionPnlUsd (https://github.com/gmx-io/gmx-synthetics/blob/b8fb11349eb59ae48a1834c239669d4ad63a38b5/contracts/position/PositionUtils.sol#L176) and includes the following code:

if (position.sizeInUsd() == sizeDeltaUsd) {
cache.sizeDeltaInTokens = position.sizeInTokens();
} else {
if (position.isLong()) {
cache.sizeDeltaInTokens = Calc.roundUpDivision(position.sizeInTokens() * sizeDeltaUsd, position.sizeInUsd());
} else {
cache.sizeDeltaInTokens = position.sizeInTokens() * sizeDeltaUsd / position.sizeInUsd();
}
}
cache.positionPnlUsd = Precision.mulDiv(cache.totalPositionPnl, cache.sizeDeltaInTokens, position.sizeInTokens());
return (cache.positionPnlUsd, cache.uncappedPositionPnlUsd, cache.sizeDeltaInTokens);

We see that the calculation for long positions involves rounding up which can increase the absolute realized PnL compared to the implementation in VaultReader::willPositionCollateralBeInsufficient. When the total PnL is negative, this discrepancy leads to underestimating the amount deducted from collateral, ultimately overestimating the remaining collateral.

Numerical Example

An example for a long BTC Position:

Assume:

  • position.sizeInTokens() = 10 BTC

  • position.sizeInUsd() = 10 * 90,000 = $900,000

  • totalPositionPnl = -$50,000

  • sizeDeltaUsd = $100,000

Using the method in VaultReader::willPositionCollateralBeInsufficient:

realizedPnlUsd = (-50,000 * 100,000) / 900,000;
realizedPnlUsd = -$5,555;

Using the GMX method:

  • Calculate sizeDeltaInTokens (rounding up for long positions):

sizeDeltaInTokens = roundUpDivision(10 * 100,000, 900,000);
sizeDeltaInTokens = 1.11 BTC (rounded up to 2 BTC)
  • Calculate realized PnL using the token-based approach:

realizedPnlUsd = (-50,000 * 2) / 10;
realizedPnlUsd = -$10,000;

Impact:

The first method assumes only -$5,555 is deducted from collateral.

The second method actually deducts -$10,000.

A discrepancy of $4,445 leads to an overestimation of remaining collateral. The discrepancy is higher for tokens with higher USD values. This miscalculation can delay necessary full position closure, leading to eventual unexpected liquidations and higher losses for the users.

Impact

This discrepancy creates a risk of unintended liquidations. Because VaultReader::willPositionCollateralBeInsufficient overestimates the remaining collateral, the PerpetualVault::_createDecreasePosition function may incorrectly decide that collateral is still sufficient, when in reality, it is not. This means:

  • The position is not fully closed when it should be.

  • An actual liquidation occurs unexpectedly, causing higher losses for users.

Tools Used

Manual review.

Recommendations

Instead of computing position.basePnlUsd for position.sizeInUsd first and then scaling it by sizeDeltaUsd, we recommend computing position.basePnlUsd for sizeDeltaUsd directly (which follows the appropriate calculation) and assigning the result to realizedPnlUsd:

PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
positionKey,
prices,
- uint256(0),
+ sizeDeltaUsd,
address(0),
- true
+ false
);
+ int256 realizedPnlUsd = positionInfo.basePnlUsd;
- int256 realizedPnlUsd;
- if (positionInfo.basePnlUsd > 0) {
- realizedPnlUsd = (uint256(positionInfo.basePnlUsd) * sizeDeltaUsd / positionInfo.position.numbers.sizeInUsd).toInt256();
- } else {
- realizedPnlUsd = -(uint256(-positionInfo.basePnlUsd) * uint256(sizeDeltaUsd) / uint256(positionInfo.position.numbers.sizeInUsd)).toInt256();
}
Updates

Lead Judging Commences

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

Support

FAQs

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

Give us feedback!