Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1017-L1171

Vulnerability details

Impact

The liquidate function modifies multiple shared state variables, including s_positionBalance, s_options, s_accountPremiumOwed, s_accountPremiumGross, s_grossPremiumLast, and s_settledTokens. If multiple liquidations for the same account are triggered simultaneously, it could lead to race conditions and incorrect liquidation outcomes.

https://github.com/code-423n4/2024-04-panoptic/blob/833312ebd600665b577fbd9c03ffa0daf250ed24/contracts/PanopticPool.sol#L1017-L1171

function liquidate(
    TokenId[] calldata positionIdListLiquidator,
    address liquidatee,
    LeftRightUnsigned delegations,
    TokenId[] calldata positionIdList
) external {
    // ... ()

    int256 liquidationBonus0;
    int256 liquidationBonus1;
    int24 finalTick;
    {
        LeftRightSigned netExchanged;
        LeftRightSigned[4][] memory premiasByLeg;
        // burn all options from the liquidatee
        (netExchanged, premiasByLeg) = _burnAllOptionsFrom(
            liquidatee,
            Constants.MIN_V3POOL_TICK,
            Constants.MAX_V3POOL_TICK,
            DONOT_COMMIT_LONG_SETTLED,
            positionIdList
        );

        // ... ()
    }

    // ... ()
}

The main issue here is that the liquidate function modifies multiple shared state variables, which could lead to race conditions and incorrect liquidation outcomes if multiple liquidations for the same account are triggered simultaneously.

Proof Of Concept

If multiple liquidations for the same account are triggered simultaneously, the updates to the shared state variables (such as s_positionBalance, s_options, s_accountPremiumOwed, s_accountPremiumGross, s_grossPremiumLast, and s_settledTokens) may occur in an unpredictable order, leading to inconsistencies and potentially incorrect liquidation outcomes.

Due to the race condition, the state of the account being liquidated may be different than expected by each of the concurrent liquidation attempts. This could result in incorrect calculations of the liquidation bonus, incorrect burning of positions, and incorrect handling of the account's collateral and premia.

Tools Used

Recommended Mitigation Steps

Instead of directly modifying the shared state variables, enqueue the liquidation requests into a queue or batch. Process the enqueued liquidation requests sequentially in a separate function or by an external system.

bool private locked = false;

modifier nonReentrant() {
    require(!locked, "Reentrancy detected");
    locked = true;
    _;
    locked = false;
}

function liquidate(
    TokenId[] calldata positionIdListLiquidator,
    address liquidatee,
    LeftRightUnsigned delegations,
    TokenId[] calldata positionIdList
) external nonReentrant {
    // ... 
}

Assessed type

Other

code-423n4 / 2024-04-panoptic-findings

Liquidated account may end up in an inconsistent state, with incorrect balances, options, and premia. #441