Description

The initForSmartContract initializes the owners for a specific smart account.

function initForSmartAccount(
    address[] calldata eoaOwners
) external returns (address) {
    if (numberOfOwners[msg.sender] != 0) {
        revert AlreadyInitedForSmartAccount(msg.sender);
    }
    uint256 ownersToAdd = eoaOwners.length;
    for (uint256 i; i < ownersToAdd; ) {
        if (eoaOwners[i] == address(0))
        // ...
    }
    // ... 
}

The entries of eoaOwners are not checked against being smart contracts. The owners should never be a smart contract as that is the general constraint that the contract adheres to.

Impact

The eoaOwners could point to an address that is a smart contract.

Recommendations

We recommend checking that none of the eoaOwners entries are smart contracts.

function initForSmartAccount(
    address[] calldata eoaOwners
) external returns (address) {
    if (numberOfOwners[msg.sender] != 0) {
        revert AlreadyInitedForSmartAccount(msg.sender);
    }
    uint256 ownersToAdd = eoaOwners.length;
    for (uint256 i; i < ownersToAdd; ) {
        if (_isSmartContract(ownersToAdd[i])) revert NotEOA(ownersToAdd[i]);
        if (eoaOwners[i] == address(0))
        // ...

    }
    // ... 
}

Remediation

Biconomy provided the following response:

Unfortunately , the extcodesize opcode is banned for the userOps with initcode > 0, thus it is not possible to apply this check at the time of the deployment of smart account with this module as a first validator module.

However, it is not an issue, as it won't be possible to sign => validate such userOp => such smart account can't be deployed. If this method is called when the given module is not the only validator, the situation won't be critical as it will be possible to change the owner thru other validator.