c4-submissions commented 10 months ago

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/libraries/CallbackLib.sol#L28 https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L415

Vulnerability details

Impact

POOL_INIT_CODE_HASH should not be hardcoded as it may make it incompatible with L2 chains, most notably zksync

Proof of Concept

The function uniswapV3MintCallback and uniswapV3SwapCallback are called by the pool after minting liquidity or executing a swap.

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L407

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/SemiFungiblePositionManager.sol#L441

These function both call the function validatecallback, which checks to see if the pool is a valid uniswap V3 pool using POOL_INIT_CODE_HASH, otherwise it will revert

    /// @param sender The address initiating the callback and claiming to be a Uniswap pool
    /// @param factory The address of the canonical Uniswap V3 factory
    /// @param features The features `sender` claims to contain
    function validateCallback(
        address sender,
        address factory,
        PoolFeatures memory features
    ) internal pure {

https://github.com/code-423n4/2023-11-panoptic/blob/f75d07c345fd795f907385868c39bafcd6a56624/contracts/libraries/CallbackLib.sol#L28

Here is the POOL_INIT_CODE_HASH

https://github.com/code-423n4/2023-11-panoptic/blob/aa86461c9d6e60ef75ed5a1fe36a748b952c8666/contracts/libraries/Constants.sol#L25-L28

The mistake here is that POOL_INIT_CODE_HASH is hardcoded, because the protocol makes the assumption that POOL_INIT_CODE_HASH bytecode will be the same across all L2 chains which is false.

For reference zksync, has a different bytecode than the one hardcoded on the original Uniswap repo.

Here is the bytecode in the protocol after converting the hash to bytes:

0xe34f199b19b2b4f47f68442619d555527d244f78a3297ea89325f843f87b8b54

This is the bytecode on zksync

https://github.com/uniswap-zksync/era-uniswap-v3-periphery/blob/30f410c5c9c937c6565fad096d8475a2d18c3fab/contracts/libraries/PoolAddress.sol#L6

This difference in bytecode can result in the protocol not being able to verify whether these callbacks cam from a legitimate pool, and subsequentially causing both functions to not work, freezing core functions of the protocol.

It is worth noting that this issue will only grow as more L2 chains are created that are EVM compatible but not EVM equivalent

Tools Used

Manual Review

Recommended Mitigation Steps

Do not hardcode POOL_INIT_CODE_HASH; instead just define it during contract initialization

Assessed type

Uniswap

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #182