code-423n4 / 2023-01-biconomy-findings

6 stars 8 forks source link

Frontrunning of smart wallet deployment #518

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Detailed description of the impact of this finding.

An attacker could obtain information about the _owner and '_index' parameters to front-run the deployment of a smart wallet.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Attack Exploit could go this way

contract Attacker { WalletFactory walletFactory;

constructor(address walletFactoryAddress) public { walletFactory = WalletFactory(walletFactoryAddress); }

function attack(address _owner, address _entryPoint, address _handler, uint _index) public {

// Generate the address of the smart wallet that is going to be deployed address walletAddress = walletFactory.getAddressForCounterfactualWallet(_owner, _index);

// Deploy a new smart wallet with the same _owner and _index parameters, but with a different _handler parameter walletFactory.deployCounterFactualWallet(_owner, _entryPoint, address(this), _index);

// Check if the smart wallet was successfully deployed require(walletFactory.isWalletExist(walletAddress), "Smart wallet was not deployed"); } }

Tools Used

Manual review

Recommended Mitigation Steps

This risk could be avoided by ensuring the details about the _owner, _entryPoint, and _handler parameters are kept confidential when deploying a new smart wallet.

c4-judge commented 1 year ago

gzeon-c4 marked the issue as duplicate of #460

livingrockrises commented 1 year ago

mitigation steps could have been better as transactions can be front runned. but we confirm the issue

c4-sponsor commented 1 year ago

livingrockrises marked the issue as sponsor confirmed

c4-sponsor commented 1 year ago

livingrockrises requested judge review

c4-judge commented 1 year ago

gzeon-c4 marked the issue as partial-50

c4-judge commented 1 year ago

gzeon-c4 marked the issue as satisfactory

c4-judge commented 1 year ago

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