Liquidity Management

RootCause & Where It Happens

In the getPositionInfo function:

Here, positionInfo.basePnlUsd can be a large negative integer, meaning the final line performs:

If -basePnlUsd exceeds netValue, this subtraction underflows in Solidity 0.8+, causing a revert. Consequently, the entire read operation fails—no result is returned. It’s impossible to “view” that position via getPositionInfo() in situations where the PnL is deeply negative.

Impact

1. Denial of Service: Any on-chain calls (or off-chain scripts that rely on a transaction simulation) to getPositionInfo() will fail, preventing critical vantage into severely underwater positions.

2. Breaking Monitoring Tools: DApps or integrators that read positions for display or risk checks can unexpectedly revert, losing insight just when the position is at its most negative.

3. No Graceful Handling of Negative netValue: The function never returns a negative netValue. Instead, it reverts, providing no opportunity for the vault or other smart contracts to handle deeply negative positions in a structured manner.

Root Cause

The function tries to store everything in a uint256 despite the possibility of a net negative outcome. In typical DeFi position accounting, it is entirely possible for PnL to be more negative than all positive offsets (collateral, claimable funding, etc.), so the final net should be negative. However, the code attempts to do:

…which reverts if <large number> is bigger than netValue.

Foundry PoC

PoC test that demonstrates the vulnerability in which a highly negative PnL (expressed via a negative basePnlUsd) causes an arithmetic underflow in the _totalAmount() computation (via getPositionInfo()). In our test, we simulate a GMX position with 100 units of collateral and a basePnlUsd of –200. When the vault code attempts to subtract 200 from 100 (after scaling), it underflows and reverts. This PoC clearly shows that getPositionInfo() will revert on deeply negative PnL.

Recommended Fixes

1. Use Signed Integers for netValue

   • If the contract design expects positions that can go below zero, keep track of netValue in an int256 and return negative values gracefully.

   • The calling context can decide how to handle negative net values (e.g. display them or consider the position liquidated).

2. Graceful Underflow Handling

   • If you must keep uint256, clamp the result at 0 or revert with a clearer reason:

     if (uint256(-basePnlUsd) > netValue) {

     // Option 1: revert("Position is too negative");

     // Option 2: netValue = 0; // if you prefer not to revert, but that can mask real negativity

     }

     netValue -= uint256(-basePnlUsd);

     ​

   • This at least prevents silent underflow reverts and clarifies the negative scenario.

3. Return Negative Values at the High Level

   • Instead of a single uint256 netValue, the contract can return both a “base collateral” and a pnl in separate fields. If the net is negative, the caller can see that distinctly.