Ethereum address collision can be used to steal funds via mint and swap callbacks

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L415
https://github.com/code-423n4/2023-11-panoptic/blob/main/contracts/SemiFungiblePositionManager.sol#L449

Vulnerability details

Impact

An attacker could generate an address that passes the CallbackValidation.verifyCallback check in Uniswap’s routers swap callback, but isn't actually a Uniswap pool.

uniswapV3MintCallback and uniswapV3SwapCallback are external functions. Anyone can call them and ask the position manager to transfer funds from a user's address directly to the msg.sender address. The only access control check this call:

CallbackLib.validateCallback(msg.sender, address(FACTORY), decoded.poolFeatures);

That check can be bypassed through using a maliciously generated msg.sender address, and used to steal all balances in users' wallets approved for the position manager.

I'm submitting this as a medium, because the attack is unlikely to be profitable (at least in the near-term future) due to the computational cost required.

Proof of Concept

An attacker can mine for two sets of addresses, using powerful hardware:

1. S1: Addresses that pass the CallbackLib.validateCallback check, but don't correspond to a real deployed Uniswap pool.

2. S2: EOA for which the attacker has the private key, or smart contract addresses that have the attack code and under control of the attacker (e.g. created using the create2 call).

The set S1 is mined by selecting a valuable real token (such as USDC as token0/token1), creating different random values for the paired token (for token1/token0) address, and storing the resulting pool address values in an efficient data structure, such as the Bloom filter.

If an address is found that belongs both to S1 and S2, the attacker can steal all the approved real token balances from a user's wallet by calling the callback functions, with amount0Owed and amount1Owed set by the attacker to match the user's approved balances.

Feasibility of the attack

Ethereum address size is 160 bits. Due to the birthday paradox the complexity of the attack on the order of 2^81 (computed as 2*2^(160/2), as two sets of addresses are mined). Already today, this complexity is not out of the scope of large or state-level actors, and will get much more feasible in the future.

Similar issue was discovered and at length described in an audit for the Kyber Swap protocol:
https://github.com/sherlock-audit/2023-07-kyber-swap-judging#issue-m-2-routersol-is-vulnerable-to-address-collission
It was considered a valid issue and paid by the sponsor.

Additional information to consider:

1. Vitalik Buterin in November 2021 claims that:
   “even without address space extension, collisions today take 2^80 time to compute, and that length of time is already within range of nation-state-level actors willing to spend huge amounts of resources. For reference, the Bitcoin network performs 2^80 SHA256 hashes once every two hours.”
   https://ethresear.ch/t/what-would-break-if-we-lose-address-collision-resistance/11356

2. Mysten Labs have recently (October 2023) estimated the cost of a collision attack. They claim that “it's not a surprise that today, a 2^80 attack would cost a few million dollars”.
   https://mystenlabs.com/blog/ambush-attacks-on-160bit-objectids-addresses

Recommended Mitigation Steps

Change the validateCallback to call factory.getPool to check that msg.sender is a Uniswap pool deployed by this factory.

Assessed type

Access Control

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

dyedm1 (sponsor) confirmed

[dyedm1]

That's fair as a Medium. Uniswap's own contracts all use the computation method, but I'm all for paranoid security.

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked issue #247 as primary and marked this issue as a duplicate of 247