Uniswap / universal-router

Uniswap's Universal Router for NFT and ERC20 swapping
GNU General Public License v3.0
410 stars 210 forks source link

The upper 12 bytes of `CREATE2` pool address computed are not cleaned #290

Open shuhuiluo opened 1 year ago

shuhuiluo commented 1 year ago

In UniswapV2Library.pairForPreSorted and V3SwapRouter.computePoolAddress, the pool address is computed in pure Solidity and casted via address(uint160(uint256())). However the upper 12 bytes are not explicitly cleaned and the address is later used in solmate::SafeTransferLib.safeTransfer which is written in inline assembly, where the full 32 bytes of to are copied to memory and directly forwarded to the ERC20 contract being called. One shouldn't assume the upper bits are cleaned when casting a keccak256 hash to address.

When compiled with via_ir enabled, all tests pass. However, after making changes that resolve "stack too deep" errors and compiling without the IR pipeline, some Foundry tests failed in an ERC20.transfer. Changing safeTransfer back to the organic transfer solves the problem.

It was discovered that the dirty upper bits of a pool address are the culprit, but somehow the IR pipeline may clean the address after keccak256. Nonetheless, the upper 12 bytes of an address computed via keccak256 should be cleaned explicitly.