Panoptic - hubble's results

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L368 https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/SemiFungiblePositionManager.sol#L1468-L1470

Vulnerability details

In initializeAMMPool() there may be a combination for univ3pool which will give a poolId value of 0. Although the s_AddrToPoolIdData[univ3pool] will store a non-zero value to make a distinction between zero and uninitialized univ3pool, there is still a confusion while fetching the poolId via getPoolId(address univ3pool). This function getPoolId() will return 0 to both an initialized(with zero) poolId and for an uninitialized univ3pool.

    function getPoolId(address univ3pool) external view returns (uint64 poolId) {
        poolId = uint64(s_AddrToPoolIdData[univ3pool]);
    }

Impact

Assume two pools in UniswapV3

  IUniswapV3Pool constant USDC_WETH_5 = IUniswapV3Pool(0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640);
  IUniswapV3Pool constant USDC_WETH_30 = IUniswapV3Pool(0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8);

Both are having same token0 and token1 but different fees. Now there is a possibility that for the first univ3pool USDC_WETH_5 during initializeAMMPool() in panoptic SemiFungiblePositionManager, the poolId value returned by getPoolId() is zero. The second univ3pool USDC_WETH_30 is still not initialized in SemiFungiblePositionManager. For both the initialized and uninitialized pools, the tokenId value will be same i.e., the 64 bits of Univ3 Pool Address, as well as token0 and token1. So if a user wishes to create a new position for the uninitialized pool (USDC_WETH_30) using mintTokenizedPosition, the position gets actually created against the initialized pool(USDC_WETH_5), since all the relevant parameters are same. This is because both getPoolId(USDC_WETH_5) and getPoolId(USDC_WETH_30) returns same value of 0, which can cause confusion in tokenId structure. The same holds while using burnTokenizedPosition()

Any upstream protocol integerating with panoptic will not be able to find any such errors.

Proof of Concept

The below test code shows how both initialized(to zero) and uninitialized pool gets value of 0 from getPoolId() Assume two pools in UniswapV3

  IUniswapV3Pool constant USDC_WETH_5 = IUniswapV3Pool(0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640);
  IUniswapV3Pool constant USDC_WETH_30 = IUniswapV3Pool(0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8);

The first pool USDC_WETH_5 is initialized and USDC_WETH_30 is uninitialzed in SemiFungiblePositionManager. Also assume that this pool USDC_WETH_5 returns a poolId of 0. For this the PanopticMath.sol needs to be Mocked as below

    function getPoolId(address univ3pool) internal pure returns (uint64) {
        if (univ3pool == address(0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640)) {
           return uint64(0);
        }
        return uint64(uint160(univ3pool) >> 96);
    }

Add the below sample foundry test into SemiFungiblePositionManager.t.sol and run

function test_initializeAMMPool_zero_poolId() public {
    _cacheWorldState(USDC_WETH_5);
    sfpm.initializeAMMPool(token0, token1, fee);
    address univ3pool = V3FACTORY.getPool(token0, token1, fee);
    console.log("Initialized   univ3pool, poolId =", univ3pool, sfpm.getPoolId(univ3pool));

    _cacheWorldState(USDC_WETH_30);
    univ3pool = V3FACTORY.getPool(token0, token1, fee);
    console.log("Uninitialized univ3pool, poolId =", univ3pool, sfpm.getPoolId(univ3pool));

}

The run logs output is as follows which shows that both have getPoolId() = 0

Logs:
  Initialized   univ3pool, poolId = 0x88e6A0c2dDD26FEEb64F039a2c41296FcB3f5640 0
  Uninitialized univ3pool, poolId = 0x8ad599c3A0ff1De082011EFDDc58f1908eb6e6D8 0

Recommended Mitigation Steps

Donot allow the value of poolId equal to 0; Add a check as below in function initializeAMMPool()

Line 368        uint64 poolId = PanopticMath.getPoolId(univ3pool);
Line 369
Line 370        while ((address(s_poolContext[poolId].pool) != address(0)) || (poolId == 0)) {     // modified line
Line 371            poolId = PanopticMath.getFinalPoolId(poolId, token0, token1, fee);
Line 372        }

Assessed type

Other