sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

Jujic - SwapperFactory is suspicious of the reorg attack #128

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Jujic

false

SwapperFactory is suspicious of the reorg attack

Summary

Vulnerability Detail

The createSwapper function deploys a SwapperImpl contract using the create, where the address derivation depends only on the SwapperFactory nonce. Some of the chains (Polygon, Optimism, Arbitrum) to which the SwapperFactory will be deployed are suspicious of the reorg attack.

https://polygonscan.com/blocks_forked

Here you may be convinced that the Polygon has in practice subject to reorgs. Even more, the reorg on the picture is 1.5 minutes long. So, it is quite enough to create the SwapperImpl and transfer funds to that address, especially when someone uses a script, and not doing it by hand.

Optimistic rollups (Optimism/Arbitrum) are also suspect to reorgs since if someone finds a fraud the blocks will be reverted, even though the user receives a confirmation and already created a split.

Impact

If users rely on the address derivation in advance or try to deploy the SwapperImpl with the same address on different EVM chains, any funds sent to the split could potentially be withdrawn by anyone else. All in all, it could lead to the theft of user funds.

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-utils/src/LibClone.sol#L105

Tool used

Manual Review

Recommendation

Deploy the SwapperImpl contract via create2 with salt and msg.sender instead create.