LiquidityManagement.katanaV3MintCallback() is subject to address collision attack, allowing an attacker to steal funds from users who give approvals to the NonfungiblePositionManager.

[howlbot-integration]

Lines of code

katana-v3-contracts/src/periphery/NonfungiblePositionManager.sol#L25-L31

Vulnerability details

Proof of Concept

The LiquidityManagement.katanaV3MintCallback() function is expected to be called only by a valid pool. However, if an attacker can craft an address (let's call it B) using the create2 opcode, which passes as a pool address, the attacker can exploit this to steal user approvals and funds. Many users, for convenience, grant maximum approvals to the NonfungiblePositionManager, which leaves them vulnerable. The attacker can use their controlled contract at address B to steal funds by calling LiquidityManagement.katanaV3MintCallback() as follows:

1. Find the address B which can pass a pool address,
2. Deploy a contract using create2 on B, say contractB
3. Call katanaV3MintCallback with appropriate values for amount0Owed, amount1Owed and make sure data.payer is the address of the user we like to steal (we can check the transaction history to identify such victims who gave approvals to the NonfungiblePositionManager).

  function katanaV3MintCallback(uint256 amount0Owed, uint256 amount1Owed, bytes calldata data) external override {
    MintCallbackData memory decoded = abi.decode(data, (MintCallbackData));
    CallbackValidation.verifyCallback(factory, decoded.poolKey);

    if (amount0Owed > 0) pay(decoded.poolKey.token0, decoded.payer, msg.sender, amount0Owed);
    if (amount1Owed > 0) pay(decoded.poolKey.token1, decoded.payer, msg.sender, amount1Owed);
  }

In the following we show how such B can be found. First, let's see how the pool verification works.

CallbackValidation.verifyCallback(factory, decoded.poolKey):


 function verifyCallback(address factory, PoolAddress.PoolKey memory poolKey)
    internal
    view
    returns (IKatanaV3Pool pool)
  {
    pool = IKatanaV3Pool(PoolAddress.computeAddress(factory, poolKey));
    require(msg.sender == address(pool));
  }

 function computeAddress(address factory, PoolKey memory key) internal pure returns (address pool) {
    require(key.token0 < key.token1);
    pool = address(
      uint256(
        keccak256(
          abi.encodePacked(
            hex"ff", factory, keccak256(abi.encode(key.token0, key.token1, key.fee)), POOL_PROXY_INIT_CODE_HASH
          )
        )
      )
    );
  }
  

The verification does not check whether the caller (msg.sender) is a pool deployed by the factory or not. It only checks that caller address matches an address that can be computed by computeAddress() where key.fee can be searched in a brute-force fashion as we show below. If the attacker can find a key.fee such that the computed address happens to be controlled by the attacker, bingo, that address can be used to steal funds.

Note that as the computation of the Create2 address is done by truncating a 256 bit keccak256 hash to 160 bits, meaning 2^96 possible hashes will be mapped to the same address!

We need to find a collision between two lists:

1. brute-force different key.fee, function computeAddress will produce a list of addresses.
2. With the attacker's factory contract, brute force various salt with create2 will produce another list of addresses.

Once we find a collision B between these two lists, the attacker can deploy a contract to address B, which passes the pool verification, and then proceed to exploit approvals and steal funds as described above.

Several similar collision attacks have been reported and rewarded:

1. Arabadzhiev - The pool verification in NapierRouter is prone to collision attacks sherlock-audit/2024-01-napier-judging#111

2. [https://github.com/PUSH0 - CREATE2 address collision against an Account will allow complete draining of lending pools sherlock-audit/2023-12-arcadia-judging#59]

3. EIP-3607, which rationale is this exact attack. The EIP is in final state.

The feasibility and cost of the attack has been discussed in above reports.

Recommended Mitigation Steps

The factory should keep track of all the pools that it has deployed, the callback function can call factory to check whether ```msg.sender`` is one of those pools.

Assessed type

Invalid Validation

[thaixuandang]

The likelihood of generating such addresses is extremely low—it’s as difficult as trying to generate Satoshi’s private key. If this could be exploited, Uniswap would have been compromised long ago.

[alex-ppg]

The Warden attempts to identify an address collision-based vulnerability based on the create2 pattern. This "vulnerability" is applicable to Uniswap V3 and many of its forks, and would result in millions of paid-out bounties if it were truly considered a medium vulnerability.

As the first Sherlock judgment outlines, the memory capacity needs beyond the computational power needs are impossible right now to even attempt this attack. I do not believe these submissions are made in good faith as medium ones, and I consider the outlined issues to be valid albeit of QA (L) severity.

I would like to correct the Sponsor in the sense that the complexity is not the same as finding a single address as a list of addresses can be generated via brute-forcing.

[c4-judge]

alex-ppg changed the severity to QA (Quality Assurance)

[c4-judge]

alex-ppg marked the issue as grade-c

[khangvv]

Hi @alex-ppg and @c4-judge, we are addressing this issue in two PRs:

• feat: address C4's issue#32 ronin-chain/katana-v3-contracts#34
• feat: address C4's issue#32 ronin-chain/katana-operation-contracts#13

Could judges and wardens please take a look?

[c4-sponsor]

@khangvv Sponsors are not allowed to close, reopen, or assign issues or pull requests.

[alex-ppg]

Hey @khangvv, I have reviewed the associated PRs and can confirm that the reported issue has been remediated in both the katana-operation-contracts and katana-v3-contracts repositories based on the contest's scope. To note, I advise any other infrastructure in use by the Ronin Chain team that was not in the scope of the contest to be updated accordingly if it employs similar checks as a best practice.

[khangvv]

Thank you, Alex.