Liquidity Management
Gamma
DeFiFoundry
50,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

`_withdraw` does not charge negative PnL correctly

novaman33

Summary

The _withdraw function incorrectly will use the position's whole PnL instead of the user's proportional PnL. This will result in incorrectly overcharging users and inability to withdraw in some cases.

Vulnerability Details

The _withdraw function will try to deduct the user's proportional PnL from the collateral being withdrawn:

int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaInUsd);
if (pnl < 0) {
collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
}

Here pnl is supposed to be only the pnl portion for the user. This is why sizeDeltaInUsd is passed which is meant to reflect the user's portion of the position size:

uint256 sizeDeltaInUsd = positionData.sizeInUsd * shares / totalShares;

However when we check out the vaultReader::getPnl function we see that the following call to the gmxReader is made:

PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
key,
prices,
sizeDeltaUsd,
address(0),
>>> true
);

However, here we can see that there is a hardcoded true for the usePositionSizeAsSizeDeltaUsd:

function getPositionInfo(
DataStore dataStore,
IReferralStorage referralStorage,
bytes32 positionKey,
MarketUtils.MarketPrices memory prices,
uint256 sizeDeltaUsd,
address uiFeeReceiver,
>>> bool usePositionSizeAsSizeDeltaUsd
) public view returns (ReaderPositionUtils.PositionInfo memory) {

As a result the getPnl will return the whole position Pnl instead of the Pnl corresponding to the user's share:

if (usePositionSizeAsSizeDeltaUsd) {
sizeDeltaUsd = positionInfo.position.sizeInUsd();
}

In case of a negative Pnl this will result in the user paying for the whole negative Pnl of the position here:
https://github.com/CodeHawks-Contests/2025-02-gamma/blob/84b9da452fc84762378481fa39b4087b10bab5e0/contracts/PerpetualVault.sol#L1112
In case the negative Pnl of the protocol is more than the position, users might be unable to withdraw during an active position, due to underflow.

Impact

Users who withdraw during an active position, will be overcharged in case of a negative Pnl.

Tools Used

Manual Review

Recommendations

Users should pay the negative Pnl based on their share, not the whole Pnl of the protocol's position in the gmx.

Updates

Lead Judging Commences

n0kto Lead Judge 7 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.