Vulnerability in Proxy Creation Method: Susceptibility to Reorg Attacks

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf552593c0d5c1552511ce724812fd6507b06368445b26b40604d28d680bf6087 Severity: low

Description: Description There is a vulnerability within the ProxyFactory contract's method of creating new proxy contracts. The _createProxy function uses the new keyword to deploy new Proxy contracts, which internally utilizes the create opcode. This approach makes the contract creation process susceptible to reorganization (reorg) attacks, particularly on Layer 2 (L2) solutions and EVM-compatible chains that are more prone to reorgs.

The vulnerability arises because the address of the newly created proxy contract is determined solely by the nonce of the factory contract. During a blockchain reorganization, this nonce can change, potentially resulting in the proxy being deployed at an unexpected address.

Attack Scenario

1. A user initiates a transaction to create a new proxy contract via the ProxyFactory.
2. Before the transaction is finalized, a blockchain reorganization occurs.
3. The ProxyFactory contract's nonce may change due to the reorg.
4. When the network stabilizes, the proxy contract might be deployed at a different address than initially expected.
5. Any subsequent transactions or interactions intended for the proxy contract may fail or interact with the wrong contract.

This scenario can lead to:

• Inconsistencies in contract deployments
• Potential loss of funds sent to an unexpected address
• Disruption of contract interactions and dependencies

Attachments

1. Proof of Concept

   
   // SPDX-License-Identifier: AGPL-3.0-only
   pragma solidity >=0.8.4;

import "./Proxy.sol";

contract ProxyFactory { event ProxyCreation(Proxy proxy, address masterCopy);

function _createProxy(address masterCopy, bytes memory data) internal returns (Proxy proxy) {

@> proxy = new Proxy(masterCopy); // Vulnerable line if (data.length > 0) { assembly { if eq(call(gas(), proxy, 0, add(data, 0x20), mload(data), 0, 0), 0) { revert(0, 0) } } } emit ProxyCreation(proxy, masterCopy); } }

2. **Revised Code File**
```diff
// SPDX-License-Identifier: AGPL-3.0-only
pragma solidity >=0.8.4;

import "./Proxy.sol";

contract ProxyFactory {
    event ProxyCreation(Proxy proxy, address masterCopy);

-   function _createProxy(address masterCopy, bytes memory data) internal returns (Proxy proxy) {
-       proxy = new Proxy(masterCopy);
+   function _createProxy(address masterCopy, bytes memory data, bytes32 salt) internal returns (Proxy proxy) {
+       proxy = new Proxy{salt: keccak256(abi.encodePacked(msg.sender, salt))}(masterCopy);
        if (data.length > 0) {
            assembly {
                if eq(call(gas(), proxy, 0, add(data, 0x20), mload(data), 0, 0), 0) { revert(0, 0) }
            }
        }
        emit ProxyCreation(proxy, masterCopy);
    }
}

The revised code demonstrates the use of create2 for proxy contract deployment. By using create2 with a salt that includes msg.sender, we ensure that the proxy contract's address will be consistent even in the event of a blockchain reorganization, as long as the same salt is used by the same sender. This significantly reduces the risk associated with reorg attacks during proxy contract deployment.

[benjaminbollen]

Thank you for your report on the proxy creation method. After review, we've determined this is not an issue.

The scenario described is not applicable. Our contract deployment process is secure.

We appreciate your participation in this security review.

[Naties29]

@benjaminbollen Hey again Ben, would like to escalate this as well. The fix for this issue revolves around very minimal change and eliminates a lot of risk that your protocol takes on board. What makes this scenario not applicable for circles?

[benjaminbollen]

normal create is not insecure

[benjaminbollen]

also see #7 ; the whole "reorg attack" is false