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

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

Open
shuhuiluo opened this issue May 7, 2023 · 0 comments · May be fixed by #291
Open

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

shuhuiluo opened this issue May 7, 2023 · 0 comments · May be fixed by #291

Comments

@shuhuiluo
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant