Skip to content

Latest commit

 

History

History
167 lines (106 loc) · 6.25 KB

File metadata and controls

167 lines (106 loc) · 6.25 KB

Mammoth Mocha Koala

Medium

Margin Check Uses Outdated Collateral Value

Summary

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/InvariantLib.sol#L19C5-L93C14

The Margin Check Uses Outdated Collateral Value issue occurs when the system verifies whether a position has sufficient collateral to remain solvent but uses the collateral value before applying changes from the new order. This allows users to withdraw collateral excessively, leaving the position undercollateralized after the update.

Root Cause

https://github.com/sherlock-audit/2025-01-perennial-v2-4-update/blob/main/perennial-v2/packages/core/contracts/libs/InvariantLib.sol#L19C5-L93C14

function validate(
    IMarket.Context memory context,
    IMarket.UpdateContext memory updateContext,
    Order memory newOrder,
    Guarantee memory newGuarantee
) external {
    // emit created event first due to early return
    emit IMarket.OrderCreated(
        context.account,
        newOrder,
        newGuarantee,
        updateContext.liquidator,
        updateContext.orderReferrer,
        updateContext.guaranteeReferrer
    );


    if (
        context.pendingLocal.invalidation != 0 &&                              // pending orders are partially invalidatable
        context.pendingLocal.neg().gt(context.latestPositionLocal.magnitude()) // total pending close is greater than latest position
    ) revert IMarket.MarketOverCloseError();


    if (newOrder.protected() && !_validateProtection(context, newOrder))
        revert IMarket.MarketInvalidProtectionError();


    if (
        !(context.latestPositionLocal.magnitude().isZero() && context.pendingLocal.isEmpty()) &&    // sender has no position
        !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO)) &&                         // sender is depositing zero or more into account, without position change
        (
            !context.latestOracleVersion.valid ||
            context.currentTimestamp - context.latestOracleVersion.timestamp >= context.riskParameter.staleAfter
        )                                                                                           // price is not stale
    ) revert IMarket.MarketStalePriceError();


    if (context.marketParameter.closed && newOrder.increasesPosition())
        revert IMarket.MarketClosedError();


    if (
        updateContext.currentPositionGlobal.maker.gt(context.riskParameter.makerLimit) &&
        newOrder.increasesMaker()
    ) revert IMarket.MarketMakerOverLimitError();


    if (!updateContext.currentPositionLocal.singleSided()) revert IMarket.MarketNotSingleSidedError();


    if (
        (!context.latestPositionLocal.maker.isZero() && !updateContext.currentPositionLocal.skew().isZero()) ||
        (!context.latestPositionLocal.skew().isZero() && !updateContext.currentPositionLocal.maker.isZero())
    ) revert IMarket.MarketNotSingleSidedError();


    if (context.pendingLocal.invalidation != 0 && context.pendingLocal.crossesZero())
        revert IMarket.MarketNotSingleSidedError();


    if (newGuarantee.priceDeviation(context.latestOracleVersion.price).gt(context.marketParameter.maxPriceDeviation))
        revert IMarket.MarketIntentPriceDeviationError();


    if (newOrder.protected()) return; // The following invariants do not apply to protected position updates (liquidations)


    if (
        !updateContext.signer &&                                            // sender is relaying the account's signed intention
        !updateContext.operator &&                                          // sender is operator approved for account
        !(newOrder.isEmpty() && newOrder.collateral.gte(Fixed6Lib.ZERO))    // sender is depositing zero or more into account, without position change
    ) revert IMarket.MarketOperatorNotAllowedError();


    if (
        context.global.currentId > context.global.latestId + context.marketParameter.maxPendingGlobal ||
        context.local.currentId > context.local.latestId + context.marketParameter.maxPendingLocal
    ) revert IMarket.MarketExceedsPendingIdLimitError();


    if (
        !PositionLib.margined(
            updateContext.currentPositionLocal.magnitude(),
            context.latestOracleVersion,
            context.riskParameter,
            updateContext.collateralization,
            context.local.collateral.add(newGuarantee.priceAdjustment(context.latestOracleVersion.price)) // apply price override adjustment from intent if present
        )

Margin Check Logic:

The margin check ensures:

PositionLib.margined( updateContext.currentPositionLocal.magnitude(), context.latestOracleVersion, context.riskParameter, updateContext.collateralization, context.local.collateral.add(newGuarantee.priceAdjustment(...)) // ❌ Uses outdated collateral ) Here, context.local.collateral is the collateral balance before applying newOrder.collateral (e.g., a withdrawal).

newOrder.collateral represents a change (e.g., withdrawal) to the collateral balance. If this delta is not included in the margin check, the calculation uses an outdated value, ignoring the impact of the current order.

Example Scenario Initial State:

Current collateral: 100

Required margin: 50

New order: Withdraw 80 collateral (newOrder.collateral = -80).

Erroneous Check:

The margin check uses context.local.collateral = 100 (pre-withdrawal value).

Calculation: 100 ≥ 50 → check passes.

Result: Collateral after withdrawal = 20 (insufficient for the required 50 margin).

Internal Pre-conditions

No response

External Pre-conditions

No response

Attack Path

No response

Impact

The protocol allows withdrawals that should be blocked, risking insolvency. The position becomes undercollateralized, violating system invariants.

PoC

No response

Mitigation

Include the newOrder.collateral delta in the margin check to use the updated collateral value:

// Corrected Code context.local.collateral .add(newOrder.collateral) // Include the collateral delta from the new order .add(newGuarantee.priceAdjustment(...))