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

Unchecked Negative PnL Handling in VaultReader Can Block Critical Protocol Operations

Summary and Impact

The VaultReader contract's getPositionInfo() function fails to properly handle negative PnL values during netValue calculations, which can prevent users from withdrawing funds and block critical protocol operations like liquidations. This directly violates the protocol's core invariant of "Depositor Share Value Preservation" as stated in the documentation, where "the value of a depositor's shares should never decrease due to the actions of other depositors."

When positions experience significant losses, the contract's inability to process large negative PnL values effectively locks user funds and prevents the protocol from managing risk properly. This is particularly concerning for a protocol designed to handle leveraged trading positions up to 3x, where significant price movements are expected.

Vulnerability Details

The issue lies in the arithmetic operation within getPositionInfo() where netValue is calculated:

// VaultReader.sol
uint256 netValue =
positionInfo.position.numbers.collateralAmount * prices.shortTokenPrice.min +
positionInfo.fees.funding.claimableLongTokenAmount * prices.longTokenPrice.min +
positionInfo.fees.funding.claimableShortTokenAmount * prices.shortTokenPrice.min -
positionInfo.fees.borrowing.borrowingFeeUsd -
positionInfo.fees.funding.fundingFeeAmount * prices.shortTokenPrice.min -
positionInfo.fees.positionFeeAmount * prices.shortTokenPrice.min;
if (positionInfo.basePnlUsd >= 0) {
netValue = netValue + uint256(positionInfo.basePnlUsd);
} else {
netValue = netValue - uint256(-positionInfo.basePnlUsd); // This line can revert
}

Consider this scenario:

  1. A user deposits 1000 USDC into a 3x ETH vault

  2. ETH price drops significantly (>33%)

  3. The position's PnL becomes deeply negative

  4. When trying to process a withdrawal or liquidation:

    • getPositionInfo() is called

    • netValue calculation attempts to subtract a large negative PnL value

    • The transaction reverts due to underflow

    • The position becomes effectively frozen

This creates a situation where:

  • Users cannot withdraw their remaining funds

  • The protocol cannot liquidate underwater positions

  • Risk management becomes impossible

  • The protocol's state becomes inconsistent with its documented invariants

Test demonstrating the issue:

function testLargeNegativePnLBlocksOperations() public {
// Setup a position with 1000 USDC collateral
uint256 collateral = 1000e6;
// Simulate ETH price drop causing large negative PnL
int256 largePnl = -1500e30; // Greater than collateral value
MarketPrices memory prices = MarketPrices({
indexTokenPrice: Price({
min: 2000e30,
max: 2000e30
}),
longTokenPrice: Price({
min: 2000e30,
max: 2000e30
}),
shortTokenPrice: Price({
min: 1e6,
max: 1e6
})
});
// This reverts, blocking withdrawals/liquidations
vm.expectRevert();
vaultReader.getPositionInfo(bytes32(uint256(1)), prices);
}

Tools Used

  • Manual Review

  • Foundry

Recommendations

Modify the netValue calculation to use signed integers (int256) throughout the calculation to properly handle negative values:

int256 netValue = int256(
positionInfo.position.numbers.collateralAmount * prices.shortTokenPrice.min
) + ... // Convert rest of calculation to signed integers
netValue = netValue + positionInfo.basePnlUsd;

This ensures the protocol can handle negative PnL values properly, maintain position management capabilities, and uphold its stated invariants regarding depositor share value preservation and withdrawal functionality.

Updates

Lead Judging Commences

n0kto Lead Judge 3 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
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.