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.
The vulnerability originates in the following portion of the VaultReader::willPositionCollateralBeInsufficient function:
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:
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:
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.
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:
Using the GMX method:
Calculate sizeDeltaInTokens (rounding up for long positions):
Calculate realized PnL using the token-based approach:
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.
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.
Manual review.
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:
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.
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.