Frontrunning DoS of SafeDeployer functions (deployConsoleAccount() and deploySubAccount())

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-brahma/blob/main/contracts/src/core/SafeDeployer.sol#L56-L255

Vulnerability details

Impact

Any console account or sub account deployment can be DoSed; an attacker constantly exploiting this vulnerability upon protocol deployment could cause full DoS of the protocol. Alternatively, an attacker can prevent further deployment of console accounts and sub accounts. Victims will also lose all gas provided for their deployment transactions.

This vulnerability report is primarily for Ethereum Mainnet, but it can also be exploited on other chains where frontrunning is possible.

Proof of Concept

In SafeDeployer.sol, the arguments passed to GnosisProxyFactory.createProxyWithNonce() are completely predetermined by the address of the sender and the arguments passed to deployConsoleAccount()/deploySubAccount(). This includes the case where address collision occurs, as SafeDeployer.sol uses an incrementing nonce in the salt to handle collision. The address of the deployed safe is also completely predetermined with these arguments (combined with the address of the GnosisProxyFactory and the contract code, but these two factors are not relevant here). Therefore, an attacker can frontrun a call to deployConsoleAccount() or deploySubAccount() with calls to GnosisProxyFactory.createProxyWithNonce() using identical arguments, causing repeated address collision in the victim's call to deployConsoleAccount()/deploySubAccount() such that the transaction is guaranteed to run out of gas and revert. There is a significant gas cost associated with failed Safe deployments due to address collision, which will be discussed in the next section.

For reference if desired, GnosisProxyFactory code from etherscan (Safe: Proxy Factory 1.3.0 https://etherscan.io/address/0xa6b71e26c5e0845f74c812102ca7114b6a896ab2#code):

    function deployProxyWithNonce(
        address _singleton,
        bytes memory initializer,
        uint256 saltNonce
    ) internal returns (GnosisSafeProxy proxy) {
        // If the initializer changes the proxy address should change too. Hashing the initializer data is cheaper than just concatinating it
        bytes32 salt = keccak256(abi.encodePacked(keccak256(initializer), saltNonce));
        bytes memory deploymentData = abi.encodePacked(type(GnosisSafeProxy).creationCode, uint256(uint160(_singleton)));
        // solhint-disable-next-line no-inline-assembly
        assembly {
            proxy := create2(0x0, add(0x20, deploymentData), mload(deploymentData), salt)
        }
        require(address(proxy) != address(0), "Create2 call failed");
    }

    function createProxyWithNonce(
        address _singleton,
        bytes memory initializer,
        uint256 saltNonce
    ) public returns (GnosisSafeProxy proxy) {
        proxy = deployProxyWithNonce(_singleton, initializer, saltNonce);
        if (initializer.length > 0)
            // solhint-disable-next-line no-inline-assembly
            assembly {
                if eq(call(gas(), proxy, 0, add(initializer, 0x20), mload(initializer), 0, 0), 0) {
                    revert(0, 0)
                }
            }
        emit ProxyCreation(proxy, _singleton);
    }

Gas Cost Analysis

GnosisProxyFactory uses the CREATE2 opcode, the details of which can be found here: https://eips.ethereum.org/EIPS/eip-1014. Note that this EIP states that "The CREATE2 has the same gas schema as CREATE ... CreateGas is deducted before evaluation of the resulting address and the execution of init_code". Since the Ethereum Yellowpaper lists the gas cost for the CREATE opcode as 32000 gas, the victim's transaction will use at least 32000 more gas for each address collision that occurs. However, testing in Foundry (see first test provided) and Remix IDE reveals that a failed CREATE2 transaction due to address collision consumes a huge amount of gas (well over 1 million) regardless of deployment bytecode size. For comparison, the attacker's frontrunning transaction costs around 300,000 gas (see first test provided).

The exact amount of gwei that the attacker needs to spend depends on the network fee, the max priority fee set by the user, and the amount of gwei provided by the user for the transaction. The attacker needs to set their max priority fee above the user's max priority fee such that their transaction is guaranteed priority, and enough attacking transactions need to be submitted such that the user's transaction will run out of gas; although considering the test, only one frontrunning transaction should be necessary. Since the attacker's transaction costs around 300,000 gas and a non-colliding account deployment transaction costs around 400,000 gas (see second test provided), the attacker will spend close to the same amount of transaction fees as the victim.

Even if the failed-due-to-collision CREATE2 gas cost as simulated by Foundry and Remix IDE is somehow incorrect, each frontrunning transaction will still grief at least 32000 gas from the user's transaction, as previously mentioned. The attack would still be very viable since a frontrunning transaction from the attacker costs around 300,000 gas. In this case, if the victim supplies 1 dollar worth of gwei for the deployment transaction, then the attacker can likely DoS the call for 10 dollars or less.

Tests to Demonstrate Address Collision and Gas Usage

Run the following tests in the DeployConsoleAccount.t.sol SafeDeployer_DeployConsoleAccountTest contract:

    function testDoS() public { 
        address[] memory owners = new address[](3);
        owners[0] = makeAddr("owners[0]");
        owners[1] = makeAddr("owners[1]");
        owners[2] = makeAddr("owners[2]");

        uint gasUsedByAttacker = gasleft(); //record and log gas used by attacker
        vm.prank(address(123)); // Use attacker address
        //attacker frontruns tx with call directly to SafeProxyFactory, passing same arguments
        address(0xa6B71E26C5e0845f74c812102Ca7114b6a896AB2).call(abi.encodeWithSignature('createProxyWithNonce(address,bytes,uint256)', address(0xd9Db270c1B5E3Bd161E8c8503c55cEABeE709552), hex'b63e800d00000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000002000000000000000000000000a238cbeb142c10ef7ad8442c6d1f9e89e07e77610000000000000000000000000000000000000000000000000000000000000180000000000000000000000000f48f2b2d2a534e402487b3ee7c18c33aec0fe5e400000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000030000000000000000000000009ec04f4f292adca2fa064f539c13622c1b52c34c00000000000000000000000074c7b92aba2859c96ae525a640f400de30ccb075000000000000000000000000ca5a7a8187978d6611f4e03d772f9ae9691793cb00000000000000000000000000000000000000000000000000000000000000a48d80ff0a00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000059000a2c4efe801858bf0ed06757253aefe0ef60f68e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000433ff495a0000000000000000000000000000000000000000000000000000000000000000000000', uint256(51872976573597302582219086198521676103224648860020643595669342518710604661707)));
        gasUsedByAttacker -= gasleft();
        console.log(gasUsedByAttacker); //around 300,000
        //SafeProxyCreationFailed() error will be thrown because EVM reverts with OutOfGas error (run test with -vvvv to see details)
        vm.expectRevert(abi.encodeWithSelector(SafeDeployer.SafeProxyCreationFailed.selector));
        //the below tx normally costs about 400,000 gas (see second test)
        safeDeployer.deployConsoleAccount{gas: 4_000_000}(owners, 2, bytes32(0), keccak256("salt"));
    }
    function testDoSGas() public { 
        address[] memory owners = new address[](3);
        owners[0] = makeAddr("owners[0]");
        owners[1] = makeAddr("owners[1]");
        owners[2] = makeAddr("owners[2]");
        uint gas = gasleft(); //record and log gas used if there's no attack
        safeDeployer.deployConsoleAccount(owners, 2, bytes32(0), keccak256("salt"));
        gas -= gasleft();
        console.log(gas); //about 400,000
        //nonce should be 1, since there was no frontrun
        assertEq(safeDeployer.ownerSafeCount(keccak256(abi.encode(owners))), 1);
    }

Tools Used

Remix IDE, Foundry, Etherscan

Recommended Mitigation Steps

Use an oracle (e.g. Chainlink VRF) to add a random value in the salt. This will effectively remove the attacker's ability to cause address collision.

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as high quality report

[0xad1onchain]

It is already mentioned in the create safe method natspec that frontrunning this call is a known vulnerability. In addition to it, the mitigation recommended is not viable for us in multiple ways including future chain availability, link token payment mechanisms and dependency on another 3rd party

[c4-sponsor]

0xad1onchain (sponsor) disputed

[alex-ppg]

The Wardens of all relevant issues have failed to properly describe how this attack would actually be a realistic vulnerability.

As some Wardens have correctly stated, this particular Denial-of-Service attack vector would, at-most, cause a transaction to fail out-of-gas and be unrealistically expensive for the attacker. In all cases, the attacker would have wasted a significant amount of funds to cause a transaction to fail while actually deploying wallets that the original caller can ultimately use as they would have been securely configured.

A real issue with this behaviour is cross-chain Denial-of-Service attacks for deterministic-address deployments. Let's consider the following:

• Alice wishes to deploy 10 wallets in multiple chains
• Alice deploys 10 wallets in chain A successfully
• Alice broadcasts two transactions to deploy 10 wallets in chain B (as one transaction would reach the block gas limit)
• Bob detects this and deploys 8 out of the 10 wallets of Alice
• Alice attempts to deploy her wallets via the SafeDeployer::deployConsoleAccount function, however, all her attempts will fail as the iterative loop would run out of gas before the retry nonce is incremented past 8

In the above scenario, Alice would need to interact directly with the Gnosis wallet deployer to deploy her remaining 2 wallets as she will be unable to do so via the SafeDeployer.

This scenario was not identified by any Warden and even so would still be classified as a QA (Low) vulnerability rather than an HM vulnerability.

Based on the facts that:

• No Warden elaborated or identified a way the attack is profitable or at least feasible for the attacker
• No Warden identified the sole case whereby the Denial-of-Service attack would actually impact a user and even then, the severity would be QA

I consider this and all relevant issues to be invalid.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Invalid

[aslanbekaibimov]

In all cases, the attacker would have wasted a significant amount of funds to cause a transaction to fail while actually deploying wallets that the original caller can ultimately use as they would have been securely configured.

This is true for Main Console Accounts, but not for SubAccounts. SafeDeployer is the only contract that can registerSubAccount in WalletRegistry. Thus, SubAccounts created by frontrunners are Brahma-incompatible, as they are created without touching SafeDeployer.

    function registerSubAccount(address _wallet, address _subAccount) external {
        if (msg.sender != AddressProviderService._getAuthorizedAddress(_SAFE_DEPLOYER_HASH)) revert InvalidSender();

No Warden elaborated or identified a way the attack is profitable or at least feasible for the attacker

No Warden identified the sole case whereby the Denial-of-Service attack would actually impact a user and even then, the severity would be QA

Here's what could realistically (in my opinion) happen:

On a cheap chain (such as Arbitrum), a Brahma Console-owner (victim) tries to deploy a Brahma SubAccount:

1. Victim signs the txn
2. Attacker deploys safes
3. Victim's txn reverts, burning all gas

The victim then will try again and be frontrun again. What could they do next? Either exit the protocol or reach out to Brahma support. The only recommendation for the victim would be to spend more gas and hope the frontrunner will eventually leave them alone.

Severity

2 — Med: Assets not at direct risk, but the function of the protocol or its availability could be impacted, 
or leak value with a hypothetical attack path with stated assumptions, but external requirements.

Impact: DoS of SubAccount deployments for indefinite amount of time; burning all of victim's gas.

The attacker himself will have to spend slighly less gas than the victim (because the attacker interacts with GnosisSafeProxyFactory directly).

Likelihood: There should exist an actor willing to set-up a bot and fund it, which is unlikely, but far from impossible. Their motivation would be either damaging Brahma's reputation, or DoS'ing specific users.

In my opinion (which may be biased), it does qualify as a Medium by C4 standards.

Mitigation

The sponsor has a valid concern over the integrity of frontrunner-deployed Safes. I believe, #283 provides an exhaustive argument for why using the frontrunned deployments is a safe and sufficient mitigation.

Thank you!

[alex-ppg]

Hey @aslanbekaibimov, thanks for your feedback!

In relation to your point about sub-accounts; yes, directly deployed sub-accounts would indeed be incompatible with the Brahma system. The Sponsor of the contest has specified that they do not list such wallets in their UI services as they are, by definition, incompatible.

With regard to front-running of a deployment, I believe your described scenario misses an important point and that is the retry loop within the deployment of both Console Accounts and Sub Accounts.

Even if the attacker does front-run the first deployment, the system will simply retry with an incremented nonce that will lead to a deployment at a new address. For an attacker to actually fully DoS an account deployment, they would have to deploy multiple accounts up-to the gas limit of the transaction.

The attacker will also end up wasting more gas than the user as the attacker’s safe will also be initialised as well as deployed, performing multiple SSTORE operations.

As a result of the above, my initial verdict stands. Please let me know if the above clarify your concerns or if you’d like to discuss this further.

[aslanbekaibimov]

@alex-ppg

I apologise for my unclear description of the attack. During Step 2, the frontrunner would have to compute how many try-catch cycles would be possible to execute with the gas the victim provided, and deploy that many Safes. I have described the same scenario in #152 in details, but failed to describe it in my initial comment.

Regarding the expenses, you're totally right.