`create` methods are suspicious of the reorg attack in `LeverageMacroFactory.sol`

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/LeverageMacroFactory.sol#L45

Vulnerability details

Impact

The deployNewMacro() function deploys a new macro for an owner and the owner can only operate the macro contract using the create, where the address derivation depends only on the LeverageMacroFactory.sol nonce. In ethereum, where this is deployed, Re-orgs has already been happened. For more info, check here

This issue will increase as some of the chains like Arbitrum and Polygon are suspicious of the reorg attacks.

The issue would happen when users rely on the address derivation in advance or try to deploy the position clone with the same address on different EVM chains, any funds sent to the new contract could potentially be withdrawn by anyone else. All in all, it could lead to the theft of user funds.

File: contracts/contracts/LeverageMacroFactory.sol

    function deployNewMacro(address _owner) public returns (address) {
        address addy = address(
>>          new LeverageMacroReference(
                borrowerOperations,
                activePool,
                cdpManager,
                ebtcToken,
                stETH,
                sortedCdps,
                _owner
            )
        );

        emit DeployNewMacro(_owner, addy);

        return addy;
    }

Attack Scenario Imagine that Alice deploys a new Macro, and then sends funds to it. Bob sees that the network block reorg happens and calls deployNewMacro. Thus, it creates LeverageMacroReference with an address to which Alice sends funds. Then Alices’ transactions are executed and Alice transfers funds to Bob’s controlled LeverageMacroReference.

Proof of Concept

https://github.com/code-423n4/2023-10-badger/blob/f2f2e2cf9965a1020661d179af46cb49e993cb7e/packages/contracts/contracts/LeverageMacroFactory.sol#L45

Tools Used

Manual review

Recommended Mitigation Steps

Deploy such contracts via create2 with salt that includes msg.sender

+    /// @notice Mapping to store deployer nonces for CREATE2
+    mapping(address deployer => uint256 nonce) public deployerNonces;

    function deployNewMacro(address _owner) public returns (address) {
-        address addy = address(
-          new LeverageMacroReference(
-                borrowerOperations,
-                activePool,
-                cdpManager,
-                ebtcToken,
-                stETH,
-                sortedCdps,
-                _owner
-            )
-        );

+        address addy = address(
+          new LeverageMacroReference{salt: keccak256(abi.encode(msg.sender, deployerNonces[msg.sender]++))}(
+                borrowerOperations,
+                activePool,
+                cdpManager,
+                ebtcToken,
+                stETH,
+                sortedCdps,
+                _owner
+            )
+            );

        emit DeployNewMacro(_owner, addy);

        return addy;
    }

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as duplicate of #224

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-judge]

jhsagd76 marked the issue as unsatisfactory: Invalid