PnL is incorrectly counted as collateral when determining whether to close positions automatically

Summary

GMX uses MIN_COLLATERAL_USD to ensure that there is always enough collateral remaining in a position, so that if there are sudden large gaps in prices, there is enough collateral to cover potential losses.

Vulnerability Detail

When a position is reduced, the estimate of remaining collateral includes the total P&L as part of the collateral. If a position has a lot of profit, a nefarious owner of the position can reduce the collateral to one wei of the collateral token, and let the position run.

Impact

If there is a sudden gap down in price, as is common with crypto Bart price chart formations, the user only loses one wei, but the pool incurs losses because there is no collateral to cover price decreases. Once one wei is left in the position, there is no mechanism for a keeper to reduce the position's leverage, so the only chance thereafter to close the position is when it needs to be liquidated.

Code Snippet

PnL is counted as collateral:

// File: gmx-synthetics/contracts/position/PositionUtils.sol : PositionUtils.willPositionCollateralBeSufficient()   #1

420            int256 remainingCollateralUsd = values.positionCollateralAmount.toInt256() * collateralTokenPrice.min.toInt256();
421    
422 @>         remainingCollateralUsd += values.positionPnlUsd;
423    
424            if (values.realizedPnlUsd < 0) {
425                remainingCollateralUsd = remainingCollateralUsd + values.realizedPnlUsd;
426            }
427    
428            if (remainingCollateralUsd < 0) {
429                return (false, remainingCollateralUsd);
430            }
431    
432:           int256 minCollateralUsdForLeverage = Precision.applyFactor(values.positionSizeInUsd, minCollateralFactor).toInt256();

https://github.com/sherlock-audit/2023-02-gmx/blob/main/gmx-synthetics/contracts/position/PositionUtils.sol#L412-L432

The position is only closed if that total is below the minimum collateral dollar amount.

Tool used

Manual Review

Recommendation

Do not count PnL as part of the collateral, for the purposes of determining the minimum position collateral amount. The combined value may still be useful as a separate check.

sherlock-audit / 2023-02-gmx-judging

IllIllI - PnL is incorrectly counted as collateral when determining whether to close positions automatically #154