sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

santipu_ - All factories are vulnerable to reorg attacks #121

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

santipu_

high

All factories are vulnerable to reorg attacks

Summary

All factories, including SwapperFactory, UniV3OracleFactory, and PassThroughWalletFactory, are susceptible to reorg attacks that could result in users losing funds sent to newly created contracts with these factories.

Vulnerability Detail

Chain reorganization (reorg) occurs when two blocks are published simultaneously. Short one- and two-block reorgs are common due to network latency, but longer reorgs can lead to malicious attacks. In certain EVM chains like Polygon, Optimism, and Arbitrum, chain reorgs occur more frequently, occasionally reaching five minutes, as evidenced in Polygon on February 22, 2023:

image

https://polygonscan.com/block/39599624/f?hash=0x0b7e6c5e9fbae3e2dbd114e4836b52ffb1211820bf62bbbd3ddf859dd07c0fe1

These chain reorg scenarios can cause users to lose funds sent to new addresses created by the affected factories.

All three factories use the create opcode to clone the original implementations. This derives the new implementation address based on the factory nonce. If a contract is created with these factories and the block is discarded minutes later, an attacker can call the same function to obtain the address that will receive the funds.

Here are potential reorg attack scenarios for each factory:

SwapperFactory: Bob deploys a new swapper contract using SwapperFactory and then sends funds to it. Alice notices a block reorg, calls createSwapper with herself as the owner, and her transaction is executed before Bob's. Consequently, Alice gets the address that will receive Bob's funds.

UniV3OracleFactory: Bob deploys a new UniV3OracleImpl using UniV3OracleFactory and then sets that oracle to an existing swapper contract. Alice detects a block reorg, calls createOracle with herself as the owner, and sets $defaultScaledOfferFactor to zero. Alice then can call flash on the swapper and steal all the funds.

PassThroughWalletFactory: Bob deploys a new PassThroughWalletImpl using PassThroughWalletFactory and then sends funds to it. Alice notices a block reorg, calls createPassThroughWallet with herself as the owner and $passThrough, and her transaction is executed before Bob's. Alice obtains the address and the funds sent to it.

Impact

During a block reorg, if a transaction involving any of the aforementioned factories is in the affected blocks, a malicious user can execute a transaction in place of the original one, gaining the address and possibly acquiring the funds that will later be sent to it.

Code Snippet

The three factories use the clone function, that uses the create opcode to deploy the new implementation contracts.

Factories: https://github.com/0xSplits/splits-oracle/blob/main/src/UniV3OracleFactory.sol#L46 https://github.com/0xSplits/splits-swapper/blob/main/src/SwapperFactory.sol#L43 https://github.com/0xSplits/splits-pass-through-wallet/blob/main/src/PassThroughWalletFactory.sol#L29

LibClone: https://github.com/0xSplits/splits-utils/blob/main/src/LibClone.sol#L105

Tool used

Manual Review

Recommendation

For all three factories, use the create2 function to deploy implementation contracts, incorporating msg.sender and the InitParams as the salt. This method ensures that the implementation address is derived not only from the factory nonce, effectively mitigating the reorg attack.