Premature AtomWallet Deployment Vulnerability Leading to Unauthorized Fund Withdrawal

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0x301f2aa465bc5205e1d38f7d64769bc92a3ca07464f32503718d3f282577bbfd
Severity: high

Description:
Description
In the EthMultiVault contract, there's a critical vulnerability in the atom creation and wallet deployment process. An attacker can exploit this to deploy an AtomWallet for an atom that hasn't been created yet, and potentially withdraw funds intended for legitimate atom wallets.

The key issues are:

1. The deployAtomWallet function only checks if the atomId is within the valid range, not if the atom actually exists.
2. The _createAtom function deposits funds into the atom wallet address without verifying if the wallet has been deployed by the protocol.
3. The AtomWallet contract allows the owner to execute arbitrary calls, which could be used to withdraw funds.

Attack Scenario\

1. An attacker observes the current count of atoms in the EthMultiVault.
2. The attacker calls deployAtomWallet with count + 1 as the atomId. This succeeds because the ID check in deployAtomWallet only verifies that the ID is not greater than the current count.
3. The attacker becomes the owner of this prematurely deployed AtomWallet.
4. When a legitimate user creates a new atom, the _createAtom function will deposit atomConfig.atomWalletInitialDepositAmount into the wallet address, which is now controlled by the attacker.
5. The attacker can then use the execute function of the AtomWallet to withdraw these funds.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)
   Update the deployAtomWallet function accordingly:

function deployAtomWallet(uint256 atomId) external whenNotPaused returns (address) {
    if (atomId == 0 || atomId > count || atoms[atomId].length == 0) {
        revert Errors.MultiVault_AtomDoesNotExist();
    }

    // compute salt for create2, including msg.sender
    bytes32 salt = keccak256(abi.encodePacked(atomId, msg.sender));

    // get contract deployment data
    bytes memory data = _getDeploymentData();

    address atomWallet;

    assembly {
        atomWallet := create2(0, add(data, 0x20), mload(data), salt)
    }

    if (atomWallet == address(0)) {
        revert Errors.MultiVault_DeployAccountFailed();
    }

    return atomWallet;
}

In the _createAtom function, ensure that only the atom creator can deploy the wallet:

address atomWallet = computeAtomWalletAddr(id);
if (atomWallet.code.length == 0) {
    atomWallet = deployAtomWallet(id);
}

These changes would ensure that:

Each AtomWallet address is unique to both the atom ID and the creator's address.
Only the legitimate creator of an atom can deploy its corresponding AtomWallet.
Attackers cannot predict or prematurely deploy AtomWallets for future atoms.

[mihailo-maksa]

The report claims a critical vulnerability in the atom creation and wallet deployment process, allowing an attacker to deploy an AtomWallet for an atom that hasn't been created yet.

Label: invalid

Comment:
The report lacks the necessary PoC and revised code file, which is mandatory for high-severity issues. The deterministic nature of create2 ensures users cannot deploy a valid atom wallet with an initial owner different from walletConfig.atomWarden, controlled by the protocol multisig. Consequently, others cannot execute arbitrary calls in AtomWallet as they won't be owners of the AtomWallet. Furthermore, checking if atomId is within the valid range is sufficient to ensure its existence, as atomId cannot be zero and must be less than count.