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

User withdraw does not calculate correct PnL.

pkqs90

Summary

User withdraw does not calculate correct PnL. This will lead to loss of funds.

Vulnerability Details

When users are withdrawing from the vault, if the vault has an open Gmx position, it will first calculate the PnL of the size of user's withdraw amount. If the PnL is negative, it would be deducted from the amount of collateral user withdraws. This makes sense, because the PnL loss for the entire position must be shared by all users.

However, when calculating the PnL, it sets the usePositionSizeAsSizeDeltaUsd parameter to true when calling gmxReader. This is wrong, because now it returns the entire position's PnL loss, instead of only the amount of the users.

This will cause the first withdrawer to lose a lot of funds.

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/PerpetualVault.sol#L1110

function _withdraw(uint256 depositId, bytes memory metadata, MarketPrices memory prices) internal {
...
IVaultReader.PositionData memory positionData = vaultReader.getPositionInfo(curPositionKey, prices);
uint256 collateralDeltaAmount = positionData.collateralAmount * shares / totalShares;
uint256 sizeDeltaInUsd = positionData.sizeInUsd * shares / totalShares;
// we always charge the position fee of negative price impact case.
uint256 feeAmount = vaultReader.getPositionFeeUsd(market, sizeDeltaInUsd, false) / prices.shortTokenPrice.max;
@> int256 pnl = vaultReader.getPnl(curPositionKey, prices, sizeDeltaInUsd);
if (pnl < 0) {
collateralDeltaAmount = collateralDeltaAmount - feeAmount - uint256(-pnl) / prices.shortTokenPrice.max;
} else {
collateralDeltaAmount = collateralDeltaAmount - feeAmount;
}
uint256 acceptablePrice = abi.decode(metadata, (uint256));
_createDecreasePosition(collateralDeltaAmount, sizeDeltaInUsd, beenLong, acceptablePrice, prices);
​
}

https://github.com/CodeHawks-Contests/2025-02-gamma/blob/main/contracts/VaultReader.sol#L186

function getPnl(
bytes32 key,
MarketPrices memory prices,
uint256 sizeDeltaUsd
) external view returns (int256) {
uint256 sizeInTokens = getPositionSizeInUsd(key);
if (sizeInTokens == 0) return 0;
PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
key,
prices,
sizeDeltaUsd,
address(0),
@> true
);
​
return positionInfo.pnlAfterPriceImpactUsd;
}

https://github.com/gmx-io/gmx-synthetics/blob/main/contracts/reader/ReaderPositionUtils.sol#L199-L201

function getPositionInfo(
DataStore dataStore,
IReferralStorage referralStorage,
Position.Props memory position,
MarketUtils.MarketPrices memory prices,
uint256 sizeDeltaUsd,
address uiFeeReceiver,
bool usePositionSizeAsSizeDeltaUsd
) internal view returns (PositionInfo memory) {
if (position.account() == address(0)) {
revert Errors.EmptyPosition();
}
​
PositionInfo memory positionInfo;
GetPositionInfoCache memory cache;
​
positionInfo.position = position;
cache.market = MarketStoreUtils.get(dataStore, positionInfo.position.market());
cache.collateralTokenPrice = MarketUtils.getCachedTokenPrice(positionInfo.position.collateralToken(), cache.market, prices);
​
if (usePositionSizeAsSizeDeltaUsd) {
@> sizeDeltaUsd = positionInfo.position.sizeInUsd();
}
...
}
}

Impact

First withdrawer lose a lot of funds.

Tools Used

N/A

Recommendations

Set usePositionSizeAsSizeDeltaUsd=false.

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.