Premature AtomWallet Deployment Vulnerability Leading to Unauthorized Fund Withdrawal

[hats-bug-reporter[bot]]

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.