Deny of service in SmartAccountFactory

[code423n4]

Lines of code

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

Vulnerability details

Impact

The salt used for create2 does not include information from the init method, so it is vulnerable to front-running.

Proof of Concept

it's impossible to override an existing contract in Ethereum. From EIP-684:

If a contract creation is attempted, due to either a creation transaction or the CREATE (or future CREATE2) opcode, and the destination address already has either nonzero nonce, or nonempty code, then the creation throws immediately, with exactly the same behavior as would arise if the first byte in the init code were an invalid opcode. This applies retroactively starting from genesis.

For that reason if the salt is repeated, the call fill fault.

bytes32 salt = keccak256(abi.encodePacked(_owner, address(uint160(_index))));

If the user want to deploy a wallet using deployCounterFactualWallet with the _owner=A and _index=0, an attacker can listen the memory pool and deploy with an higer gas, a wallet with the same owner and index, but with different _entryPoint and _handler so the attacker can deny the original call, and make unwanted actions in the proxy.

BaseSmartAccount(proxy).init(_owner, _entryPoint, _handler);

Recommended Mitigation Steps

Include all received fields in the salt calculation.

[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

[c4-judge]

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