hats-finance / Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a

The smart contracts of the Intuition protocol v1.
https://intuition.systems
Other
0 stars 1 forks source link

Changing `atomWarden` allows redeploying the Atom Vault wallet #51

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

Github username: @Al-Qa-qa Twitter username: al_qa_qa Submission hash (on-chain): 0xc518fa0e591487973f4d31750e421dc652b1ae07f42aee2489353a9ea1ea9f70 Severity: medium

Description: Description\ When we deploy Atom Wallets, we are not storing the deployed atomWallets. we just use create2, and if it is already deployed no thing occuar.

EthMultiVault.sol#L375-L377

    function deployAtomWallet(uint256 atomId) external whenNotPaused returns (address) {
        if (atomId == 0 || atomId > count) {
            revert Errors.MultiVault_VaultDoesNotExist();
        }

        // compute salt for create2
        bytes32 salt = bytes32(atomId);

        // get contract deployment data
        bytes memory data = _getDeploymentData();
        ...
        assembly {
            atomWallet := create2(0, add(data, 0x20), mload(data), salt)
        }
        ...
    }

The problem is that the address of the atom wallets depends on the constructor argument and atomWarden is one of these args, so in case of changing atomWarden by the protocol Devs. all the created atom wallets for vaults IDs can get redeployed again. as the address that will return from create2 will be different because of the changing constructor argument.

EthMultiVault.sol#L1421-L1423

        bytes memory initData = abi.encodeWithSelector(
            AtomWallet.init.selector, IEntryPoint(walletConfig.entryPoint), walletConfig.atomWarden, address(this)
        );

This will affect the system each Atom contains only one URI that represent the data. and the users can prove the ownership of this URI to claim the ownership of the wallet. So they will be able to claim 2 wallets instead of just one in this case.

Recommentations\ Make a mapping that stores the deployed Atom Vaults IDs, so in case of redeploying it after changing atomWarden, it will revert as it is already deployed

mihailo-maksa commented 4 days ago

Duplicate of issue #50.

Al-Qa-qa commented 3 days ago

Added info here: https://github.com/hats-finance/Intuition-0x538dbadc50cc87b281cd655f1edbc6ebda02a66a/issues/50#issuecomment-2198048391