Atom wallets can be created and attributed to Triple vaults as well due to invalid validation

[hats-bug-reporter[bot]]

Github username: -- Twitter username: iamandreiski Submission hash (on-chain): 0x73097393c0cbe8c1ed9e3b9df4b12c607b7e2a36218527a734efe18280ae9636 Severity: medium

Description: Description\ Since there's no validation besides if the atomId isn't 0 or it's not more than the count, an Atom wallet can be created/deployed for a Triple vault as well.

Attack Scenario\

When deploying a new Atom wallet, according to protocol documentation, as well as Natspec, it should be attributed to an Atom vault. But there's no contract logic preventing a wallet to be deployed for a Triple vault.

When a user wants to deploy an Atom wallet:

 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();

        address atomWallet;

        // deploy atom wallet with create2:
        // value sent in wei,
        // memory offset of `code` (after first 32 bytes where the length is),
        // length of `code` (first 32 bytes of code),
        // salt for create2
        assembly {
            atomWallet := create2(0, add(data, 0x20), mload(data), salt)
        }

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

        return atomWallet;
    }

They can pass an Id which is attributed to a Triple vault, since there's no validation to make sure that the Id passed as function argument is not a Triple vault id, the vault will be deployed as usual and attributed to the "Triple vault".

The only logic which validates this is that the vaultId isn't 0 or that it's not more than count, since count is both incremented for Triple vaults and Atom vaults, this will pass and the wallet will be created.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

[mihailo-maksa]

The reported issue concerning the possibility of creating atom wallets for Triple vaults due to insufficient validation has been reviewed. Here is our detailed perspective:

Intended Functionality: In our protocol, triples can also function as atoms (identities) within other triples. This design choice allows triples to have their own atom wallets, providing flexibility in how the protocol handles these entities.

Validation Consideration: While the current implementation does not explicitly prevent the creation of atom wallets for Triple vaults, this is not necessarily a flaw. The ability for triples to have atom wallets aligns with the intended functionality of the protocol.

Severity Assessment: Since this behavior does not introduce any security risks or functional issues and aligns with the protocol's design, it is not considered a vulnerability. It is more of an enhancement suggestion to add an additional validation step if stricter separation is desired.

Conclusion: The ability for Triple vaults to have atom wallets is an intentional feature of the protocol. However, for clarity and to avoid potential misuse, we could add an additional check to explicitly prevent the deployment of atom wallets for triple IDs if needed. This issue is best classified as an enhancement rather than a security vulnerability.

Status: This issue is an enhancement.

Comment for the Reporter: Thank you for highlighting this potential issue. While the current design allows triples to have atom wallets, which is intentional, we understand the value of adding explicit validation for clarity. This issue is best classified as an enhancement, and we will consider adding the suggested validation. We can still consider a lower payout for this valid suggestion.