DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: high
Valid

Incorrect PNL Calculation in withdraw Due to usePositionSizeAsSizeDeltaUsd Misconfiguration

Summary

The PerpetualVault contract’s withdraw function incorrectly calculates a user’s profit/loss (PNL) using the getPnl function in VaultReader, which passes true for the usePositionSizeAsSizeDeltaUsd parameter in gmxReader.getPositionInfo. This returns the total PNL for the entire position instead of the user’s proportional PNL, potentially underpaying or overpaying users during withdrawals and violating fair share distribution.

Vulnerability Details

  • In PerpetualVault’s withdraw function, PNL is calculated to adjust collateralDeltaAmount:

    int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaUsd);
    if (pnl < 0) {
    collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
    }
  • The getPnl function in VaultReader calls gmxReader.getPositionInfo with usePositionSizeAsSizeDeltaUsd = true:

    PositionInfo memory positionInfo = gmxReader.getPositionInfo(
    address(dataStore),
    referralStorage,
    key,
    prices,
    sizeDeltaUsd,
    address(0),
    true
    );
  • In gmxReader (Reader contract), usePositionSizeAsSizeDeltaUsd = true causes getPositionInfo to use the position’s full size as sizeDeltaUsd, returning the total PNL for the position (e.g., -100,000 USD for a position with a total loss).

  • However, for a withdrawal, sizeDeltaUsd should represent the user’s share of the position (e.g., 50,000 USD for a user with 50% ownership), and usePositionSizeAsSizeDeltaUsd should be false to calculate the user’s proportional PNL (e.g., -50,000 USD).

  • This misconfiguration results in incorrect PNL values, potentially deducting too much or too little collateral from the user’s withdrawal, leading to unfair distribution.

Impact

  • Financial Loss or Gain for Users: Users could be underpaid (e.g., losing 100,000 USD of collateral instead of 50,000 USD) or overpaid (e.g., receiving more collateral than entitled), disrupting fair share distribution and risking vault insolvency.

  • Unfair Share Distribution: The issue violates the README’s key invariant: "Depositor Share Value Preservation: The value of a depositor’s shares should never decrease due to the actions of other depositors. Any losses or gains should be distributed proportionally based on share ownership."

  • User Trust Erosion: Incorrect withdrawals could erode trust in the protocol, potentially leading to loss of users and reputational damage.

  • Vault Instability: Overpaying or underpaying users could deplete or inflate the vault’s collateral, risking operational instability or liquidation.

Tools Used

  • Manual code review

Recommendations

Update the getPnl function in VaultReader to pass false for usePositionSizeAsSizeDeltaUsd when calculating PNL for user-specific actions like withdrawals

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding_getPnL_use_usePositionSizeAsSizeDeltaUsd_and_substract_it

Likelihood: Medium/High, every withdrawal from a short or leveraged vault that is not liquidated and has a negative PnL. Impact: High, subtract collateralDeltaAmount from the entire PnL of the position instead of the delta amount PnL. DoS or loss of funds

Support

FAQs

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

Give us feedback!