PP-105: signature verification on createUserSmartWallet call

contracts/factory/CustomSmartWalletFactory.sol

@@ -118,20 +118,17 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {
         bytes calldata initParams,
         bytes calldata sig
     ) external override {
-        bytes memory packed =
-            abi.encodePacked(
-                "\x19\x10",
-                owner,
-                recoverer,
-                logic,
-                index,
-                initParams
-            );
+        bytes32 _hash = keccak256(abi.encodePacked(
+            address(this),
+            owner,
+            recoverer,
+            logic,
+            index,
+            initParams
+        ));
         (sig);
-        require(
-            RSKAddrValidator.safeEquals(keccak256(packed).recover(sig), owner),
-            "Invalid signature"
-        );
+        bytes32 message = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", _hash));
+        require(RSKAddrValidator.safeEquals(message.recover(sig),owner), "Invalid signature");
 
         //e6ddc71a  =>  initialize(address owner,address logic,address tokenAddr,address tokenRecipient,uint256 tokenAmount,uint256 tokenGas,bytes initParams)
         bytes memory initData =

contracts/factory/SmartWalletFactory.sol

@@ -115,14 +115,15 @@ contract SmartWalletFactory is ISmartWalletFactory {
         uint256 index,
         bytes calldata sig
     ) external override {
-        bytes memory packed = abi.encodePacked(
-            "\x19\x10",
+        bytes32 _hash = keccak256(abi.encodePacked(
+            address(this),
             owner,
             recoverer,
             index
-        );
-
-        require(RSKAddrValidator.safeEquals(keccak256(packed).recover(sig),owner), "Invalid signature");
+        ));
+        (sig);
+        bytes32 message = keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", _hash));
+        require(RSKAddrValidator.safeEquals(message.recover(sig),owner), "Invalid signature");
 
         //a6b63eb8  =>  initialize(address owner,address tokenAddr,address tokenRecipient,uint256 tokenAmount,uint256 tokenGas)  
         bytes memory initData = abi.encodeWithSelector(

contracts/smartwallet/CustomSmartWallet.sol

@@ -112,6 +112,8 @@ contract CustomSmartWallet is IForwarder {
 
        bytes32 logicStrg;
         assembly {
+            // The logic contract address
+            // IMPLEMENTATION_SLOT = bytes32(uint256(keccak256('eip1967.proxy.implementation')) - 1)
             logicStrg := sload(
                 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc
             )

contracts/smartwallet/SmartWallet.sol

@@ -10,6 +10,9 @@ import "../utils/RSKAddrValidator.sol";
 /* solhint-disable avoid-low-level-calls */
 
 contract SmartWallet is IForwarder {
+
+    //slot for owner = bytes32(uint256(keccak256('eip1967.proxy.owner')) - 1) = a7b53796fd2d99cb1f5ae019b54f9e024446c3d12b483f733ccc62ed04eb126a
+    bytes32 private constant _OWNER_SLOT = 0xa7b53796fd2d99cb1f5ae019b54f9e024446c3d12b483f733ccc62ed04eb126a;
     using ECDSA for bytes32;
 
     uint256 public override nonce;
@@ -43,7 +46,7 @@ contract SmartWallet is IForwarder {
 
         assembly {
             sstore(
-                0xa7b53796fd2d99cb1f5ae019b54f9e024446c3d12b483f733ccc62ed04eb126a,
+                _OWNER_SLOT,
                 ownerCell
             )
         }
@@ -60,7 +63,7 @@ contract SmartWallet is IForwarder {
     function getOwner() private view returns (bytes32 owner){
         assembly {
             owner := sload(
-                0xa7b53796fd2d99cb1f5ae019b54f9e024446c3d12b483f733ccc62ed04eb126a
+                _OWNER_SLOT
             )
         }
     }
@@ -149,7 +152,7 @@ contract SmartWallet is IForwarder {
         if(req.tokenAmount > 0){
             (success, ret) = req.tokenContract.call{gas: req.tokenGas}(
                 abi.encodeWithSelector(
-                    hex"a9059cbb", 
+                    hex"a9059cbb", //transfer(address,uint256)
                     feesReceiver,
                     req.tokenAmount
                 )
@@ -272,7 +275,7 @@ contract SmartWallet is IForwarder {
         //we need to initialize the contract
         if (tokenAmount > 0) {
             (bool success, bytes memory ret ) = tokenAddr.call{gas: tokenGas}(abi.encodeWithSelector(
-                hex"a9059cbb",
+                hex"a9059cbb", //transfer(address,uint256) 
                 tokenRecipient,
                 tokenAmount));
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@rsksmart/rif-relay-contracts",
-  "version": "1.0.0-alpha.3",
+  "version": "1.0.0-alpha.4",
   "description": "This project contains all the contracts needed for the rif relay system.",
   "main": "dist/index.js",
   "private": false,

test/factory/CustomSmartWalletFactory.test.ts

@@ -1,6 +1,5 @@
 import { use, expect } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
-import { ethers } from 'ethers';
 import { soliditySha3Raw } from 'web3-utils';
 import {
     CustomSmartWalletFactoryInstance,
@@ -16,6 +15,7 @@ import {
     mintTokens,
     signRequest
 } from '../utils';
+import { createValidPersonalSignSignature } from './utils';
 
 use(chaiAsPromised);
 
@@ -29,6 +29,7 @@ type createUserSmartWalletParam = {
     logicAddress: string;
     index: string;
     initParams: string;
+    factoryAddress: string;
 };
 
 /**
@@ -45,23 +46,24 @@ function createUserSmartWalletSignature(
     ownerPrivateKey: Buffer,
     object: createUserSmartWalletParam
 ): string {
-    const { owner, recoverer, logicAddress, index, initParams } = object;
-
-    const toSign: string =
-        web3.utils.soliditySha3(
-            { t: 'bytes2', v: '0x1910' },
-            { t: 'address', v: owner },
-            { t: 'address', v: recoverer },
-            { t: 'address', v: logicAddress },
-            { t: 'uint256', v: index },
-            { t: 'bytes', v: initParams }
-        ) ?? '';
-    const toSignAsBinaryArray = ethers.utils.arrayify(toSign);
-    const signingKey = new ethers.utils.SigningKey(ownerPrivateKey);
-    const signature = signingKey.signDigest(toSignAsBinaryArray);
-    const signatureCollapsed = ethers.utils.joinSignature(signature);
-
-    return signatureCollapsed;
+    const {
+        owner,
+        recoverer,
+        logicAddress,
+        index,
+        initParams,
+        factoryAddress
+    } = object;
+
+    const message = web3.utils.soliditySha3(
+        { t: 'address', v: factoryAddress },
+        { t: 'address', v: owner },
+        { t: 'address', v: recoverer },
+        { t: 'address', v: logicAddress },
+        { t: 'uint256', v: index },
+        { t: 'bytes', v: initParams }
+    );
+    return createValidPersonalSignSignature(ownerPrivateKey, message);
 }
 
 contract('CustomSmartWalletFactory', ([worker, otherAccount]) => {
@@ -104,7 +106,8 @@ contract('CustomSmartWalletFactory', ([worker, otherAccount]) => {
                     recoverer,
                     logicAddress,
                     initParams,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 
@@ -131,7 +134,8 @@ contract('CustomSmartWalletFactory', ([worker, otherAccount]) => {
                     recoverer,
                     logicAddress,
                     initParams,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 
@@ -157,7 +161,8 @@ contract('CustomSmartWalletFactory', ([worker, otherAccount]) => {
                     recoverer,
                     logicAddress,
                     initParams,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 

test/factory/smartWalletFactory.test.ts

@@ -1,7 +1,6 @@
 import { AccountKeypair } from '@rsksmart/rif-relay-client';
 import { use, expect } from 'chai';
 import chaiAsPromised from 'chai-as-promised';
-import { ethers } from 'ethers';
 import {
     SmartWalletFactoryInstance,
     SmartWalletInstance,
@@ -16,6 +15,7 @@ import {
     mintTokens,
     signRequest
 } from '../utils';
+import { createValidPersonalSignSignature } from './utils';
 
 use(chaiAsPromised);
 
@@ -27,6 +27,7 @@ type createUserSmartWalletParam = {
     owner: string;
     recoverer: string;
     index: string;
+    factoryAddress: string;
 };
 
 /**
@@ -43,21 +44,14 @@ function createUserSmartWalletSignature(
     ownerPrivateKey: Buffer,
     object: createUserSmartWalletParam
 ): string {
-    const { owner, recoverer, index } = object;
-
-    const toSign: string =
-        web3.utils.soliditySha3(
-            { t: 'bytes2', v: '0x1910' },
-            { t: 'address', v: owner },
-            { t: 'address', v: recoverer },
-            { t: 'uint256', v: index }
-        ) ?? '';
-    const toSignAsBinaryArray = ethers.utils.arrayify(toSign);
-    const signingKey = new ethers.utils.SigningKey(ownerPrivateKey);
-    const signature = signingKey.signDigest(toSignAsBinaryArray);
-    const signatureCollapsed = ethers.utils.joinSignature(signature);
-
-    return signatureCollapsed;
+    const { owner, recoverer, index, factoryAddress } = object;
+    const message = web3.utils.soliditySha3(
+        { t: 'address', v: factoryAddress },
+        { t: 'address', v: owner },
+        { t: 'address', v: recoverer },
+        { t: 'uint256', v: index }
+    );
+    return createValidPersonalSignSignature(ownerPrivateKey, message);
 }
 
 contract('SmartWalletFactory', ([worker, otherAccount]) => {
@@ -92,7 +86,8 @@ contract('SmartWalletFactory', ([worker, otherAccount]) => {
                 {
                     owner: owner.address,
                     recoverer,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 
@@ -116,7 +111,8 @@ contract('SmartWalletFactory', ([worker, otherAccount]) => {
                 {
                     owner: constants.ZERO_ADDRESS,
                     recoverer,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 
@@ -138,7 +134,8 @@ contract('SmartWalletFactory', ([worker, otherAccount]) => {
                 {
                     owner: owner.address,
                     recoverer,
-                    index
+                    index,
+                    factoryAddress: factory.address
                 }
             );
 

test/factory/utils.ts

@@ -0,0 +1,28 @@
+import { ethers } from 'ethers';
+
+const PERSONAL_SIGN_PREFIX = '\x19Ethereum Signed Message:\n';
+
+const stringOrEmpty = (str: string | null) => str ?? '';
+
+export const createValidPersonalSignSignature = (
+    ownerPrivateKey: Buffer,
+    msg: string | null
+) => {
+    const message = stringOrEmpty(msg);
+    const toSign = stringOrEmpty(
+        web3.utils.soliditySha3(
+            {
+                t: 'string',
+                v: PERSONAL_SIGN_PREFIX + web3.utils.hexToBytes(message).length
+            },
+            { t: 'bytes32', v: message }
+        )
+    );
+
+    const toSignAsBinaryArray = ethers.utils.arrayify(toSign);
+    const signingKey = new ethers.utils.SigningKey(ownerPrivateKey);
+    const signature = signingKey.signDigest(toSignAsBinaryArray);
+    const signatureCollapsed = ethers.utils.joinSignature(signature);
+
+    return signatureCollapsed;
+};