Attacker could deploy contract in the name of a user with his own parameters

[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

SCW factory's main purpose is to deploy contracts for addresses through deployWallet and deployCounterFactualWallet. In the case of deployCounterFactualWallet, deployed contract's address depends on the salt which is provided in the arguments. Functions however don't take into account who the msg.caller is, which means consequentially anyone can deploy a contact for anyone and initialize it with his own parameters. It is true that user can just deploy another contract with a different index, but user might not know that this even happened.

When a user wants to deploy a contract, an attacker can front-run him and deploy a contract with having himself as the entrypoint, which more or less grants him access to the contract through execFromEntryPoint(). This would also mislead the user into thinking he actually deployed the contract.

The second danger here would be that since the factory also has a getAddressForCounterfactualWallet() function, the user could deposit funds to a non yet existing contract and an attacker could create a contract and take the funds by making himself entryPoint and then having access to execute() function.

Proof of Concept

1. User submits a deployCounterFactualWallet() transaction
2. Attacker frontrunns him and creates the contract having himself as _entryPoint.
3. Users transaction failed because the contract already exists with the given salt, so the user might think he created it sometime before, so he continues to use it.
4. At some point attacker steals all the funds from the contract using execute()

Tools Used

Manual review

Recommended Mitigation Steps

Only allow the actual owner and protocol contracts to deploy contracts for the owner.

[c4-judge]

gzeon-c4 marked the issue as duplicate of #460

[c4-sponsor]

livingrockrises marked the issue as sponsor confirmed

[c4-judge]

gzeon-c4 marked the issue as satisfactory