Accept only string for signing (#215)

* don't take bytes, use hashMessage from ethers * add changeset * devDep bump * include dist files
safe-global · Sep 9, 2021 · 8777635 · 8777635
1 parent 1aa48d0
commit 8777635
Show file tree

Hide file tree

Showing 19 changed files with 418 additions and 406 deletions.
diff --git a/.changeset/lemon-pants-itch.md b/.changeset/lemon-pants-itch.md
@@ -0,0 +1,5 @@
+---
+'@gnosis.pm/safe-apps-sdk': patch
+---
+
+Don't accept bytes for signing, use hashMessage from ethers (eip-191)
diff --git a/package.json b/package.json
@@ -19,13 +19,13 @@
     "lerna": "^4.0.0"
   },
   "devDependencies": {
-    "@typescript-eslint/eslint-plugin": "^4.29.3",
-    "@typescript-eslint/parser": "^4.29.3",
+    "@typescript-eslint/eslint-plugin": "^4.31.0",
+    "@typescript-eslint/parser": "^4.31.0",
     "eslint": "7.32.0",
     "eslint-config-prettier": "^8.3.0",
     "eslint-plugin-prettier": "^4.0.0",
-    "jest": "27.1.0",
-    "prettier": "^2.3.2",
+    "jest": "27.1.1",
+    "prettier": "^2.4.0",
     "ts-jest": "^27.0.5",
     "typescript": "^4.4.2"
   }

diff --git a/packages/safe-apps-sdk/dist/package.json b/packages/safe-apps-sdk/dist/package.json
@@ -1,6 +1,6 @@
 {
     "name": "@gnosis.pm/safe-apps-sdk",
-    "version": "4.3.0-next.0",
+    "version": "4.3.0-next.1",
     "description": "SDK developed to integrate third-party apps with Safe-Multisig app.",
     "main": "dist/src/index.js",
     "typings": "dist/src/index.d.ts",
@@ -27,9 +27,9 @@
     },
     "devDependencies": {
         "@types/jest": "^27.0.1",
-        "@types/node": "^16.7.2",
-        "@typescript-eslint/eslint-plugin": "^4.29.3",
-        "@typescript-eslint/parser": "^4.29.3",
+        "@types/node": "^16.9.0",
+        "@typescript-eslint/eslint-plugin": "^4.31.0",
+        "@typescript-eslint/parser": "^4.31.0",
         "rimraf": "^3.0.2"
     },
     "repository": {

diff --git a/packages/safe-apps-sdk/dist/src/safe/index.d.ts b/packages/safe-apps-sdk/dist/src/safe/index.d.ts
@@ -1,12 +1,12 @@
-import { Communicator, SafeInfo, SafeBalances, GetBalanceParams, BytesLike } from '../types';
+import { Communicator, SafeInfo, SafeBalances, GetBalanceParams } from '../types';
 declare class Safe {
     private readonly communicator;
     constructor(communicator: Communicator);
     getInfo(): Promise<SafeInfo>;
     experimental_getBalances({ currency }?: GetBalanceParams): Promise<SafeBalances>;
     private check1271Signature;
     private check1271SignatureBytes;
-    isMessageSigned(message: BytesLike, signature?: string): Promise<boolean>;
+    isMessageSigned(message: string, signature?: string): Promise<boolean>;
     isMessageHashSigned(messageHash: string, signature?: string): Promise<boolean>;
 }
 export { Safe };
diff --git a/packages/safe-apps-sdk/dist/src/safe/index.js.map b/packages/safe-apps-sdk/dist/src/safe/index.js.map
diff --git a/packages/safe-apps-sdk/dist/src/safe/signatures.d.ts b/packages/safe-apps-sdk/dist/src/safe/signatures.d.ts
@@ -1,8 +1,7 @@
 import { ethers } from 'ethers';
-import { BytesLike } from '../types';
 declare const MAGIC_VALUE = "0x1626ba7e";
 declare const MAGIC_VALUE_BYTES = "0x20c13b0b";
 declare const EIP_1271_INTERFACE: ethers.utils.Interface;
 declare const EIP_1271_BYTES_INTERFACE: ethers.utils.Interface;
-declare const calculateMessageHash: (message: BytesLike) => string;
+declare const calculateMessageHash: (message: string) => string;
 export { EIP_1271_INTERFACE, EIP_1271_BYTES_INTERFACE, MAGIC_VALUE, MAGIC_VALUE_BYTES, calculateMessageHash };
diff --git a/packages/safe-apps-sdk/dist/src/safe/signatures.js b/packages/safe-apps-sdk/dist/src/safe/signatures.js
diff --git a/packages/safe-apps-sdk/dist/src/safe/signatures.js.map b/packages/safe-apps-sdk/dist/src/safe/signatures.js.map
diff --git a/packages/safe-apps-sdk/dist/src/txs/index.d.ts b/packages/safe-apps-sdk/dist/src/txs/index.d.ts
@@ -1,9 +1,9 @@
-import { GatewayTransactionDetails, SendTransactionsParams, Communicator, SendTransactionsResponse, BytesLike } from '../types';
+import { GatewayTransactionDetails, SendTransactionsParams, Communicator, SendTransactionsResponse } from '../types';
 declare class TXs {
     private readonly communicator;
     constructor(communicator: Communicator);
     getBySafeTxHash(safeTxHash: string): Promise<GatewayTransactionDetails>;
-    signMessage(message: BytesLike): Promise<SendTransactionsResponse>;
+    signMessage(message: string): Promise<SendTransactionsResponse>;
     send({ txs, params }: SendTransactionsParams): Promise<SendTransactionsResponse>;
 }
 export { TXs };
diff --git a/packages/safe-apps-sdk/dist/src/txs/index.js.map b/packages/safe-apps-sdk/dist/src/txs/index.js.map
diff --git a/packages/safe-apps-sdk/dist/src/types/sdk.d.ts b/packages/safe-apps-sdk/dist/src/types/sdk.d.ts
@@ -3,8 +3,6 @@ export declare type BaseTransaction = {
     value: string;
     data: string;
 };
-export declare type Bytes = ArrayLike<number>;
-export declare type BytesLike = Bytes | string;
 export declare type GetTxBySafeTxHashParams = {
     safeTxHash: string;
 };
@@ -19,7 +17,7 @@ export declare type GetBalanceParams = {
     currency?: string;
 };
 export declare type SignMessageParams = {
-    message: BytesLike;
+    message: string;
 };
 export declare type SendTransactionsResponse = {
     safeTxHash: string;

diff --git a/packages/safe-apps-sdk/package.json b/packages/safe-apps-sdk/package.json
@@ -27,9 +27,9 @@
   },
   "devDependencies": {
     "@types/jest": "^27.0.1",
-    "@types/node": "^16.7.2",
-    "@typescript-eslint/eslint-plugin": "^4.29.3",
-    "@typescript-eslint/parser": "^4.29.3",
+    "@types/node": "^16.9.0",
+    "@typescript-eslint/eslint-plugin": "^4.31.0",
+    "@typescript-eslint/parser": "^4.31.0",
     "rimraf": "^3.0.2"
   },
   "repository": {

diff --git a/packages/safe-apps-sdk/src/safe/index.ts b/packages/safe-apps-sdk/src/safe/index.ts
@@ -8,15 +8,7 @@ import {
 } from './signatures';
 import { Methods } from '../communication/methods';
 import { RPC_CALLS } from '../eth/constants';
-import {
-  Communicator,
-  SafeInfo,
-  SafeBalances,
-  GetBalanceParams,
-  RPCPayload,
-  TransactionConfig,
-  BytesLike,
-} from '../types';
+import { Communicator, SafeInfo, SafeBalances, GetBalanceParams, RPCPayload, TransactionConfig } from '../types';
 
 class Safe {
   private readonly communicator: Communicator;
@@ -108,7 +100,7 @@ class Safe {
     }
   }
 
-  async isMessageSigned(message: BytesLike, signature = '0x'): Promise<boolean> {
+  async isMessageSigned(message: string, signature = '0x'): Promise<boolean> {
     const messageHash = calculateMessageHash(message);
     const messageHashSigned = await this.isMessageHashSigned(messageHash, signature);
 

diff --git a/packages/safe-apps-sdk/src/safe/safe.test.ts b/packages/safe-apps-sdk/src/safe/safe.test.ts
@@ -44,7 +44,7 @@ describe('Safe Apps SDK safe methods', () => {
       // to test message/hash I signed a test message on rinkeby
       // https://dashboard.tenderly.co/tx/rinkeby/0x9308fb61d9f4282080334e3f35b357fc689e06808b8ad2817536813948e3720d
       const message = 'approve rugpull';
-      const expectedHash = '0xb4fd0d8fd75eea963cec570dd58d8c3f5f93569f5c112e227fa64f275623b4db';
+      const expectedHash = '0xe32c44147e358bc973757518210c3baec92de66115c513ea1146d61ad4fd93af';
       const hash = calculateMessageHash(message);
 
       expect(hash).toEqual(expectedHash);
@@ -219,7 +219,7 @@ describe('Safe Apps SDK safe methods', () => {
 
       // ethers.utils.formatBytes32String('approve rugpull')
       const message = '0x617070726f76652072756770756c6c0000000000000000000000000000000000';
-      const expectedHash = '0xdbdc6aad7abc8ee87185f03191abf6b43ea347f0e669f829ab1af0ee59bae246';
+      const expectedHash = '0xaae9257b8ff1c926ac3cdf36923661de4e81bf934e38958beeede3519aa18b08';
 
       sdkInstance.safe.isMessageSigned(message);
       expect(isMessageHashSignedSpy).toHaveBeenCalledWith(expectedHash, '0x');

diff --git a/packages/safe-apps-sdk/src/safe/signatures.ts b/packages/safe-apps-sdk/src/safe/signatures.ts
@@ -1,5 +1,4 @@
 import { ethers } from 'ethers';
-import { BytesLike } from '../types';
 
 const MAGIC_VALUE = '0x1626ba7e';
 const MAGIC_VALUE_BYTES = '0x20c13b0b';
@@ -11,12 +10,8 @@ const EIP_1271_BYTES_INTERFACE = new ethers.utils.Interface([
   'function isValidSignature(bytes calldata _data, bytes calldata _signature) public view',
 ]);
 
-const calculateMessageHash = (message: BytesLike): string => {
-  if (typeof message === 'string') {
-    message = ethers.utils.toUtf8Bytes(message);
-  }
-
-  return ethers.utils.keccak256(message);
+const calculateMessageHash = (message: string): string => {
+  return ethers.utils.hashMessage(message);
 };
 
 export { EIP_1271_INTERFACE, EIP_1271_BYTES_INTERFACE, MAGIC_VALUE, MAGIC_VALUE_BYTES, calculateMessageHash };
diff --git a/packages/safe-apps-sdk/src/txs/index.ts b/packages/safe-apps-sdk/src/txs/index.ts
@@ -6,7 +6,6 @@ import {
   GetTxBySafeTxHashParams,
   Communicator,
   SendTransactionsResponse,
-  BytesLike,
 } from '../types';
 
 class TXs {
@@ -30,7 +29,7 @@ class TXs {
     return response.data;
   }
 
-  async signMessage(message: BytesLike): Promise<SendTransactionsResponse> {
+  async signMessage(message: string): Promise<SendTransactionsResponse> {
     const messagePayload = {
       message,
     };

diff --git a/packages/safe-apps-sdk/src/types/sdk.ts b/packages/safe-apps-sdk/src/types/sdk.ts
@@ -4,10 +4,6 @@ export type BaseTransaction = {
   data: string;
 };
 
-export type Bytes = ArrayLike<number>;
-
-export type BytesLike = Bytes | string;
-
 export type GetTxBySafeTxHashParams = {
   safeTxHash: string;
 };
@@ -24,7 +20,7 @@ export interface SendTransactionsParams {
 export type GetBalanceParams = { currency?: string };
 
 export type SignMessageParams = {
-  message: BytesLike;
+  message: string;
 };
 
 export type SendTransactionsResponse = {

diff --git a/packages/safe-apps-web3-react/package.json b/packages/safe-apps-web3-react/package.json
@@ -31,7 +31,7 @@
   },
   "devDependencies": {
     "@types/jest": "^27.0.1",
-    "@types/node": "^16.7.2",
+    "@types/node": "^16.9.0",
     "@web3-react/core": "6.1.9",
     "react": "17.0.2",
     "rimraf": "^3.0.2"