🐛 Includes CallData in Deployment Signature

Prevent a tricky attack involving a bundler.

script/AccountFactory/02_FactoryCreateAndInitAccount.s.sol

@@ -8,22 +8,35 @@ import { BaseScript } from "../Base.s.sol";
 /// @notice Create and init an account using an already deployed factory
 /// @dev    The signature must be signed by the admin of the factory
 contract FactoryCreateAndInitAccount is BaseScript {
-    /// @notice Deploy an account and init it
-    /// @return accountAddress The address of the deployed account
     function run() public broadcast returns (address accountAddress) {
         address factoryAddress = vm.envAddress("FACTORY");
-        AccountFactory factory = AccountFactory(factoryAddress);
-
-        // arguments to pass to the `createAndInit` function
         bytes memory authData = vm.envBytes("AUTH_DATA");
         bytes memory signature = vm.envBytes("SIGNATURE");
+        bytes memory callData = vm.envBytes("CALL_DATA");
+
+        return run(factoryAddress, authData, signature, callData);
+    }
+
+    /// @notice Deploy an account and init it
+    /// @return accountAddress The address of the deployed account
+    function run(
+        address factoryAddress,
+        bytes memory authData,
+        bytes memory signature,
+        bytes memory callData
+    )
+        internal
+        broadcast
+        returns (address accountAddress)
+    {
+        AccountFactory factory = AccountFactory(factoryAddress);
 
         // check the account is not already deployed
         accountAddress = factory.getAddress(authData);
         require(accountAddress.code.length == 0, "Account already exists");
 
         // deploy and init the account
-        address deployedAddress = factory.createAndInitAccount(authData, signature);
+        address deployedAddress = factory.createAndInitAccount(authData, signature, keccak256(callData));
 
         // ensure the account has been deployed at the correct address
         require(deployedAddress == accountAddress, "Invalid account address");

src/v1/Account/SmartAccount.sol

@@ -150,11 +150,13 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
     ///     deployment. The use of the account factory is gated by this signature.
     /// @param signature The signature field presents in the userOp.
     /// @param initCode The initCode field presents in the userOp. It has been used to create the account
+    /// @param callData The callData field presents in the userOp.
     /// @return 0 if the signature is valid, 1 otherwise
     function _validateCreationSignature(
         uint256 nonce,
-        bytes calldata signature,
-        bytes calldata initCode
+        bytes calldata initCode,
+        bytes calldata callData,
+        bytes calldata signature
     )
         internal
         virtual
@@ -169,22 +171,25 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
         if (accountFactory != storedFactoryAddress) return Signature.State.FAILURE;
 
         // 3. decode the rest of the initCode (skip the first 4 bytes -- function selector)
-        (bytes memory authenticatorData,) = abi.decode(initCode[24:], (bytes, bytes));
+        (bytes memory authenticatorData,,) = abi.decode(initCode[24:], (bytes, bytes, bytes32));
 
         // 4. extract the signer from the authenticatorData
         // TODO: once tested, rework this shit by using a more efficient way
         (, bytes32 credIdHash, uint256 pubX, uint256 pubY) =
             SmartAccount(payable(address(this))).extractSignerFromAuthData(authenticatorData);
 
         // 5. recreate the message and try to recover the signer
-        bytes memory message = abi.encode(Signature.Type.CREATION, authenticatorData, address(this), block.chainid);
+        bytes memory message =
+            abi.encode(Signature.Type.CREATION, authenticatorData, address(this), block.chainid, keccak256(callData));
 
         // 6. fetch the expected signer from the factory contract
         address expectedSigner = AccountFactory(storedFactoryAddress).owner();
 
         // 7. Check the signature is valid and revert if it is not
         // NOTE: The signature prefix, added manually to identify the signature, is removed before the recovery process
-        if (Signature.recover(expectedSigner, message, signature[1:]) == false) return Signature.State.FAILURE;
+        if (Signature.recover(expectedSigner, message, signature[1:]) == false) {
+            return Signature.State.FAILURE;
+        }
 
         // 8. Ensure the signer is allowed. This is the signer added by the factory during the deployment process.
         // solhint-disable-next-line var-name-mixedcase
@@ -249,7 +254,7 @@ contract SmartAccount is Initializable, UUPSUpgradeable, BaseAccount, SmartAccou
 
         // 1.b check the signature is a "creation" signature (length is checked by the signature library)
         if (signatureType == Signature.Type.CREATION) {
-            return _validateCreationSignature(userOp.nonce, userOp.signature, userOp.initCode);
+            return _validateCreationSignature(userOp.nonce, userOp.initCode, userOp.callData, userOp.signature);
         }
 
         return Signature.State.FAILURE;

src/v1/AccountFactory.sol

@@ -32,7 +32,7 @@ contract AccountFactory {
 
     event AccountCreated(address account, bytes authenticatorData);
 
-    error InvalidSignature(address accountAddress, bytes authenticatorData, bytes signature);
+    error InvalidSignature(address accountAddress, bytes32 callDataHash, bytes authenticatorData, bytes signature);
     error InvalidAccountImplementation();
     error InvalidSigner();
 
@@ -62,12 +62,14 @@ contract AccountFactory {
 
     /// @notice This function checks if the signature is signed by the operator
     /// @param  accountAddress The address of the account that would be deployed
+    /// @param  callDataHash The hash of the calldata of the transaction that would be sent to the account
     /// @param  authenticatorData The authenticatorData field of the WebAuthn response when creating a signer
     /// @param  signature Signature made off-chain by made the operator of the factory. It gates the use of the factory.
     /// @return True if the signature is legit, false otherwise
     /// @dev    Incorrect signatures are expected to lead to a revert by the library used
     function _isSignatureLegit(
         address accountAddress,
+        bytes32 callDataHash,
         bytes calldata authenticatorData,
         bytes calldata signature
     )
@@ -77,7 +79,9 @@ contract AccountFactory {
         returns (bool)
     {
         // 1. Recreate the message signed by the operator
-        bytes memory message = abi.encode(Signature.Type.CREATION, authenticatorData, accountAddress, block.chainid);
+        // NOTE: msg.sender is assumed to be the expected entrypoint
+        bytes memory message =
+            abi.encode(Signature.Type.CREATION, authenticatorData, accountAddress, block.chainid, callDataHash);
 
         // 2. Try to recover the address and return if the signature is legit
         return Signature.recover(operator, message, signature[1:]);
@@ -168,7 +172,8 @@ contract AccountFactory {
     /// @return The address of the existing account (either deployed by this function or not)
     function createAndInitAccount(
         bytes calldata authenticatorData,
-        bytes calldata signature
+        bytes calldata signature,
+        bytes32 callDataHash
     )
         external
         virtual
@@ -185,8 +190,8 @@ contract AccountFactory {
         if (accountAddress.code.length > 0) return accountAddress;
 
         // 4. check if the signature is valid
-        if (_isSignatureLegit(accountAddress, authenticatorData, signature) == false) {
-            revert InvalidSignature(accountAddress, authenticatorData, signature);
+        if (_isSignatureLegit(accountAddress, callDataHash, authenticatorData, signature) == false) {
+            revert InvalidSignature(accountAddress, callDataHash, authenticatorData, signature);
         }
 
         // 5. deploy the proxy for the user. During the deployment, the initialize function in the implementation

test/BaseTest/BaseTest.sol

@@ -21,18 +21,41 @@ contract BaseTest is Test, BaseTestUtils, BaseTestCreateFixtures {
     ///      This utility function is used to craft a signature for the deployment of an account.
     ///      This is for testing purposes only.
     /// @param authenticatorData The authenticator data returned by the authenticator on the signer creation
-    /// @param account The address of the account that will be deployed
+    /// @param accountAddress The address of the account that will be deployed
     /// @return signature The signature to be used for the deployment
     function craftDeploymentSignature(
         bytes memory authenticatorData,
-        address account
+        address accountAddress
+    )
+        internal
+        view
+        returns (bytes memory signature)
+    {
+        return craftDeploymentSignature(authenticatorData, accountAddress, createFixtures.transaction.calldataHash);
+    }
+
+    /// @notice Utility function to craft a deployment signature
+    /// @dev In production, the deployment of an account using our factory is gated by an approval from us.
+    ///      The factory will check if smoo.th approved the deployment by verifying a signature
+    ///      we create using an approved signer (the operator and also the owner of the factory).
+    ///      This utility function is used to craft a signature for the deployment of an account.
+    ///      This is for testing purposes only.
+    /// @param authenticatorData The authenticator data returned by the authenticator on the signer creation
+    /// @param accountAddress The address of the account that will be deployed
+    /// @param calldataHash The hash of the calldata that will be executed after the deployment
+    /// @return signature The signature to be used for the deployment
+    function craftDeploymentSignature(
+        bytes memory authenticatorData,
+        address accountAddress,
+        bytes32 calldataHash
     )
         internal
         view
         returns (bytes memory signature)
     {
         // recreate the message to sign
-        bytes memory message = abi.encode(Signature.Type.CREATION, authenticatorData, account, block.chainid);
+        bytes memory message =
+            abi.encode(Signature.Type.CREATION, authenticatorData, accountAddress, block.chainid, calldataHash);
 
         // hash the message with the EIP-191 prefix
         bytes32 hash = MessageHashUtils.toEthSignedMessageHash(message);

test/BaseTest/BaseTestCreateFixtures.sol

@@ -28,12 +28,18 @@ struct FixturesResponse {
     bytes authData;
 }
 
+struct FixturesTransaction {
+    bytes callData;
+    bytes32 calldataHash;
+}
+
 struct CreateFixtures {
     uint256 id;
     FixturesUser user;
     FixturesSignature signature;
     FixturesSigner signer;
     FixturesResponse response;
+    FixturesTransaction transaction;
 }
 
 /// @title BaseTestCreateFixtures
@@ -90,6 +96,7 @@ contract BaseTestCreateFixtures is Test {
         FixturesSignature memory signature;
         FixturesSigner memory signer;
         FixturesResponse memory response;
+        FixturesTransaction memory transaction;
 
         // load and store the user data
         {
@@ -137,14 +144,23 @@ contract BaseTestCreateFixtures is Test {
             });
         }
 
+        // craft the dummy alldata passed during the deployment
+        {
+            bytes memory callData =
+                abi.encodeWithSignature("transfer(address,address,uint256)", address(234), address(500), 1);
+            bytes32 calldataHash = keccak256(callData);
+            transaction = FixturesTransaction({ callData: callData, calldataHash: calldataHash });
+        }
+
         // create the fixture and store it in the storage
         // forgefmt: disable-next-item
         createFixtures = CreateFixtures({
             id: id,
             user: user,
             signature: signature,
             signer: signer,
-            response: response
+            response: response,
+            transaction: transaction
         });
     }
 }

test/unit/v1/Account/SmartAccountERC1271/EIP191.t.sol

@@ -69,7 +69,11 @@ contract SmartAccountERC1271__EIP191 is BaseTest {
 
         // 8. deploy the proxy that targets the implementation and set the first signer using the creationAuthData
         bytes memory signature = craftDeploymentSignature(data.creationAuthData, accountFutureAddress);
-        account = SmartAccount(payable(factory.createAndInitAccount(data.creationAuthData, signature)));
+        account = SmartAccount(
+            payable(
+                factory.createAndInitAccount(data.creationAuthData, signature, createFixtures.transaction.calldataHash)
+            )
+        );
     }
 
     function test_CanValidateEIP191Signature() external {

test/unit/v1/Account/SmartAccountERC1271/EIP712.t.sol

@@ -71,7 +71,11 @@ contract SmartAccountERC1271__EIP712 is BaseTest {
 
         // 8. deploy the proxy that targets the implementation and set the first signer using the creationAuthData
         bytes memory signature = craftDeploymentSignature(data.creationAuthData, accountFutureAddress);
-        account = SmartAccount(payable(factory.createAndInitAccount(data.creationAuthData, signature)));
+        account = SmartAccount(
+            payable(
+                factory.createAndInitAccount(data.creationAuthData, signature, createFixtures.transaction.calldataHash)
+            )
+        );
     }
 
     function _getDomainSeparator() internal pure returns (bytes32) {

test/unit/v1/Account/addWebAuthnP256R1Signer.t.sol

@@ -31,7 +31,13 @@ contract SmartAccount__AddWebAuthnP256R1Signer is BaseTest {
 
         // 5. deploy the proxy that targets the implementation and set the first signer
         bytes memory signature = craftDeploymentSignature(createFixtures.response.authData, accountFutureAddress);
-        account = SmartAccount(payable(factory.createAndInitAccount(createFixtures.response.authData, signature)));
+        account = SmartAccount(
+            payable(
+                factory.createAndInitAccount(
+                    createFixtures.response.authData, signature, createFixtures.transaction.calldataHash
+                )
+            )
+        );
     }
 
     function test_RevertsIfTheSignerAlreadyExists() external {

test/unit/v1/Account/getSigner.t.sol

@@ -27,7 +27,13 @@ contract SmartAccount__GetSigner is BaseTest {
         bytes memory signature = craftDeploymentSignature(createFixtures.response.authData, accountFutureAddress);
 
         // 3. deploy the proxy that targets the implementation and set the first signer
-        return SmartAccount(payable(factory.createAndInitAccount(createFixtures.response.authData, signature)));
+        return SmartAccount(
+            payable(
+                factory.createAndInitAccount(
+                    createFixtures.response.authData, signature, createFixtures.transaction.calldataHash
+                )
+            )
+        );
     }
 
     function test_ReturnsTheStoredSignerWhenPassingCredId() external {

test/unit/v1/Account/removeWebAuthnP256R1Signer.t.sol

@@ -28,7 +28,13 @@ contract SmartAccount__RemoveWebAuthnP256R1Signer is BaseTest {
 
         // 5. deploy the proxy that targets the implementation and set the first signer
         bytes memory signature = craftDeploymentSignature(createFixtures.response.authData, accountFutureAddress);
-        account = SmartAccount(payable(factory.createAndInitAccount(createFixtures.response.authData, signature)));
+        account = SmartAccount(
+            payable(
+                factory.createAndInitAccount(
+                    createFixtures.response.authData, signature, createFixtures.transaction.calldataHash
+                )
+            )
+        );
     }
 
     function test_CanOnlyBeCalledByItself(address caller) external {

test/unit/v1/Account/upgradeable.t.sol

@@ -30,7 +30,13 @@ contract SmartAccount__Upgradeable is BaseTest {
         bytes memory signature = craftDeploymentSignature(createFixtures.response.authData, accountFutureAddress);
 
         // 6. deploy an account instance and set the first signer
-        account = SmartAccount(payable(factory.createAndInitAccount(createFixtures.response.authData, signature)));
+        account = SmartAccount(
+            payable(
+                factory.createAndInitAccount(
+                    createFixtures.response.authData, signature, createFixtures.transaction.calldataHash
+                )
+            )
+        );
     }
 
     function test_CanBeUpgradedWithoutData() external {