deployCounterFactualWallet does not check whether the caller is the owner

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccountFactory.sol#L33

Vulnerability details

Impact

Detailed description of the impact of this finding
deployCounterFactualWallet(() does not check whether the caller is the owner

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.
https://github.com/code-423n4/2023-01-biconomy/blob/53c8c3823175aeb26dee5529eeefa81240a406ba/scw-contracts/contracts/smart-contract-wallet/SmartAccountFactory.sol#L33

deployCounterFactualWallet(() does not check whether the caller is the owner, as a result, any user might this function and create a SmartAccunt for another user.

Tools Used

Remix

Recommended Mitigation Steps

We can add a check in the beginning of the function.

function deployCounterFactualWallet(address _owner, address _entryPoint, address _handler, uint _index) public returns(address proxy){

        if(owner != msg.sender) revert NotByOwner();

        bytes32 salt = keccak256(abi.encodePacked(_owner, address(uint160(_index))));
        bytes memory deploymentData = abi.encodePacked(type(Proxy).creationCode, uint(uint160(_defaultImpl)));
        // solhint-disable-next-line no-inline-assembly
        assembly {
            proxy := create2(0x0, add(0x20, deploymentData), mload(deploymentData), salt)
        }
        require(address(proxy) != address(0), "Create2 call failed");
        // EOA + Version tracking
        emit SmartAccountCreated(proxy,_defaultImpl,_owner, VERSION, _index);
        BaseSmartAccount(proxy).init(_owner, _entryPoint, _handler);
        isAccountExist[proxy] = true;
    }

[c4-judge]

gzeon-c4 marked the issue as duplicate of #460

[livingrockrises]

this is not the intended functionality (eoa owners may not have gas to do so) and there are better solutions to prevent this/front running.

[c4-sponsor]

livingrockrises marked the issue as sponsor disputed

[c4-judge]

gzeon-c4 changed the severity to 3 (High Risk)

[c4-judge]

gzeon-c4 marked the issue as satisfactory