Token Sorting Vulnerability in SemiFungiblePositionManager.sol

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L350-L391

Vulnerability details

Impact

The absence of token sorting in the initializeAMMPool() function can lead to inconsistencies in Automated Market Maker (AMM) pools, with the order of token0 and token1 varying depending on the chain. This inconsistency can cause issues in subsequent operations involving these tokens, affecting trading, liquidity provision, and other pool-related activities. If pools are initialized incorrectly, it could lead to miscalculations or improper behavior within the protocol.

Proof of Concept

• The code from SemiFungiblePositionManager.sol that initializes AMM pools without sorting tokens can lead to the order of token0 and token1 being unpredictable.
• This issue may cause complications when pools are initialized from a Panoptic pool or directly by calling initializeAMMPool().
• The inconsistency between token ordering might not be immediately visible but could result in errors when interacting with these pools in the future.

    function initializeAMMPool(address token0, address token1, uint24 fee) external {
        // compute the address of the Uniswap v3 pool for the given token0, token1, and fee tier
        address univ3pool = FACTORY.getPool(token0, token1, fee);

        // reverts if the Uni v3 pool has not been initialized
        if (univ3pool == address(0)) revert Errors.UniswapPoolNotInitialized();

        // return if the pool has already been initialized in SFPM
        // @dev pools can be initialized from a Panoptic pool or by calling initializeAMMPool directly, reverting
        // would prevent any PanopticPool from being deployed
        // @dev some pools may not be deployable if the poolId has a collision (since we take only 8 bytes)
        // if poolId == 0, we have a bit on the left set if it was initialized, so this will still return properly
        if (s_AddrToPoolIdData[univ3pool] != 0) return;

        // The base poolId is composed as follows:
        // [tickSpacing][pool pattern]
        // [16 bit tickSpacing][most significant 48 bits of the pool address]
        uint64 poolId = PanopticMath.getPoolId(univ3pool);

        // There are 281,474,976,710,655 possible pool patterns.
        // A modern GPU can generate a collision such a space relatively quickly,
        // so if a collision is detected increment the pool pattern until a unique poolId is found
        while (address(s_poolContext[poolId].pool) != address(0)) {
            poolId = PanopticMath.incrementPoolPattern(poolId);
        }

        // store the poolId => UniswapV3Pool information in a mapping
        // `locked` being initialized to false is gas-efficient because the pool address makes the slot nonzero
        // note: we preserve the state of `locked` to prevent reentering a pool by initializing it during the reentrant call
        s_poolContext[poolId] = PoolAddressAndLock({
            pool: IUniswapV3Pool(univ3pool),
            locked: s_poolContext[poolId].locked
        });

        // store the UniswapV3Pool => poolId information in a mapping
        // add a bit on the end to indicate that the pool is initialized
        // (this is for the case that poolId == 0, so we can make a distinction between zero and uninitialized)
        unchecked {
            s_AddrToPoolIdData[univ3pool] = uint256(poolId) + 2 ** 255;
        }
        emit PoolInitialized(univ3pool, poolId);
    }

Tools Used

The following tools were used to identify and document the bug: Manual Review

Recommended Mitigation Steps

Sort token0 and token1 within the initializeAMMPool() function to ensure consistency in AMM pools across different chains. This could be as simple as checking and swapping the tokens if they are in the wrong order.

Assessed type

Context

[Picodes]

https://docs.uniswap.org/contracts/v3/reference/core/interfaces/IUniswapV3Factory#getpool

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid