DeFiFoundry
50,000 USDC
View results
Submission Details
Severity: low
Invalid

Incorrect Collateral Insufficiency Calculation Leading to Erroneous Liquidations in VaultReader.sol

Summary

The VaultReader.sol contract incorrectly calculates collateral sufficiency when determining if a position should be liquidated. The function willPositionCollateralBeInsufficient() does not accurately account for unrealized PnL (Profit & Loss) and misestimates remaining collateral after liquidating a portion of a position.

As a result, healthy positions can be liquidated prematurely or remain open when they should be liquidated, depending on market conditions.

Vulnerability Details

function willPositionCollateralBeInsufficient(
MarketPrices memory prices,
bytes32 positionKey,
address market,
bool isLong,
uint256 sizeDeltaUsd,
uint256 collateralDeltaAmount
) external view returns (bool) {
if (getPositionSizeInUsd(positionKey) == 0) return true;
PositionInfo memory positionInfo = gmxReader.getPositionInfo(
address(dataStore),
referralStorage,
positionKey,
prices,
uint256(0),
address(0),
true
);
int256 realizedPnlUsd;
if (positionInfo.basePnlUsd > 0) {
realizedPnlUsd = (uint256(positionInfo.basePnlUsd) * sizeDeltaUsd / positionInfo.position.numbers.sizeInUsd).toInt256();
} else {
realizedPnlUsd = -(uint256(-positionInfo.basePnlUsd) * uint256(sizeDeltaUsd) / uint256(positionInfo.position.numbers.sizeInUsd)).toInt256();
}
MarketUtils.WillPositionCollateralBeSufficientValues memory values = MarketUtils.WillPositionCollateralBeSufficientValues({
positionSizeInUsd: positionInfo.position.numbers.sizeInUsd - sizeDeltaUsd,
positionCollateralAmount: positionInfo.position.numbers.collateralAmount - collateralDeltaAmount,
realizedPnlUsd: realizedPnlUsd,
openInterestDelta: -int256(sizeDeltaUsd)
});
MarketProps memory marketInfo = getMarket(market);
(bool willBeSufficient, ) = MarketUtils.willPositionCollateralBeSufficient(
dataStore,
marketInfo,
prices,
isLong,
values
);
return !willBeSufficient;
}

Incorrect PnL Scaling Calculation:

realizedPnlUsd = (uint256(positionInfo.basePnlUsd) * sizeDeltaUsd / positionInfo.position.numbers.sizeInUsd).toInt256();

If the position is partially reduced, the realizedPnlUsd calculation does not correctly proportion the unrealized PnL based on the actual collateral remaining.

  • This can overestimate losses, causing unnecessary liquidations.

  • Failure to Account for Funding Fees:

    positionInfo.fees.funding.fundingFeeAmount * prices.shortTokenPrice.min
    • The funding fee deduction is static but should be dynamic based on the actual funding accrued.

  • Incorrect positionCollateralAmount Calculation:

    positionCollateralAmount: positionInfo.position.numbers.collateralAmount - collateralDeltaAmount,
    • This assumes that collateralDeltaAmount is always valid.

    • If the remaining collateral is too low, the function should trigger liquidation earlier.

Impact

Attack Vector Description Severity
Forced Liquidation of Healthy Positions Users lose positions even if they are profitable. 🔴 Critical
Vault Insolvency If liquidations are too frequent, the vault may become undercollateralized. 🔴 Critical
Excessive Funding Fees Charged If the funding fee is applied incorrectly, users might pay more than necessary. 🟠 High
Unfair Advantage for Manipulators Attackers can trigger mass liquidations by manipulating oracle prices. 🟠 High

Tools Used

Manual Review

Recommendations

  1. Correct the realizedPnlUsd Calculation

    if (positionInfo.basePnlUsd > 0) {
    realizedPnlUsd = (int256(positionInfo.basePnlUsd) * int256(sizeDeltaUsd) / int256(positionInfo.position.numbers.sizeInUsd));
    } else {
    realizedPnlUsd = -(int256(-positionInfo.basePnlUsd) * int256(sizeDeltaUsd) / int256(positionInfo.position.numbers.sizeInUsd));
    }

    Ensures proper scaling of unrealized PnL when partially reducing a position.

  2. Account for Accrued Funding Fees Dynamically

    uint256 fundingFees = getAccruedFundingFees(positionKey, sizeDeltaUsd);
    positionCollateralAmount -= fundingFees;
    • Retrieve real-time funding fees instead of using outdated values.

  3. Add a Check for Minimum Collateral Requirements

    if (positionCollateralAmount < MIN_REQUIRED_COLLATERAL) {
    return true; // Force liquidation if collateral is too low
    }
    • Ensures users are not liquidated unfairly due to rounding issues.

Updates

Lead Judging Commences

n0kto Lead Judge 9 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
Assigned finding tags:

Suppositions

There is no real proof, concrete root cause, specific impact, or enough details in those submissions. Examples include: "It could happen" without specifying when, "If this impossible case happens," "Unexpected behavior," etc. Make a Proof of Concept (PoC) using external functions and realistic parameters. Do not test only the internal function where you think you found something.

Support

FAQs

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