Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
c4-bot-4 opened this issue Oct 20, 2024 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working 🤖_primary AI based primary recommendation 🤖_37_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality

Comments

@c4-bot-4
Copy link
Contributor

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

@c4-bot-4 c4-bot-4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Oct 20, 2024
c4-bot-6 added a commit that referenced this issue Oct 20, 2024
@c4-bot-12 c4-bot-12 added 🤖_37_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation labels Oct 30, 2024
howlbot-integration bot added a commit that referenced this issue Nov 4, 2024
@howlbot-integration howlbot-integration bot added the sufficient quality report This report is of sufficient quality label Nov 4, 2024
@nevillehuang
Copy link

Likely invalid and OOS per contest details. This is likely a known uniswapv3 issue.

All public known issues, including public audit reports of Uniswap V3 that affect Katana V3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working 🤖_primary AI based primary recommendation 🤖_37_group AI based duplicate group recommendation sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants