Bug: Missing context in signature checks

.changeset/cool-ladybugs-admire.md

@@ -0,0 +1,5 @@
+---
+'@gnosis.pm/safe-apps-sdk': patch
+---
+
+fix missing context on signature checks

packages/safe-apps-sdk/dist/package.json

@@ -1,6 +1,6 @@
 {
     "name": "@gnosis.pm/safe-apps-sdk",
-    "version": "4.2.0",
+    "version": "4.3.0-next.0",
     "description": "SDK developed to integrate third-party apps with Safe-Multisig app.",
     "main": "dist/src/index.js",
     "typings": "dist/src/index.d.ts",

packages/safe-apps-sdk/dist/src/safe/index.d.ts

@@ -4,9 +4,10 @@ declare class Safe {
     constructor(communicator: Communicator);
     getInfo(): Promise<SafeInfo>;
     experimental_getBalances({ currency }?: GetBalanceParams): Promise<SafeBalances>;
-    calculateMessageHash(message: BytesLike): string;
+    static calculateMessageHash(message: BytesLike): string;
     private check1271Signature;
     private check1271SignatureBytes;
     isMessageSigned(message: BytesLike, signature?: string): Promise<boolean>;
+    isMessageHashSigned(messageHash: string, signature?: string): Promise<boolean>;
 }
 export { Safe };

packages/safe-apps-sdk/package.json

@@ -15,7 +15,7 @@
   ],
   "scripts": {
     "test": "jest",
-    "format-dist": "sed -i 's/\"files\":/\"_files\":/' dist/package.json",
+    "format-dist": "sed -i '' 's/\"files\":/\"_files\":/' dist/package.json",
     "build": "yarn rimraf dist && tsc && yarn format-dist",
     "lint": "tslint -p tsconfig.json"
   },

packages/safe-apps-sdk/src/safe/index.ts

@@ -40,15 +40,15 @@ class Safe {
     return response.data;
   }
 
-  calculateMessageHash(message: BytesLike): string {
+  static calculateMessageHash(message: BytesLike): string {
     if (typeof message === 'string') {
       message = ethers.utils.toUtf8Bytes(message);
     }
 
     return ethers.utils.keccak256(message);
   }
 
-  private async check1271Signature(messageHash: Uint8Array, signature = '0x'): Promise<boolean> {
+  private async check1271Signature(messageHash: string, signature = '0x'): Promise<boolean> {
     const safeInfo = await this.getInfo();
 
     const encodedIsValidSignatureCall = EIP_1271_INTERFACE.encodeFunctionData('isValidSignature', [
@@ -78,11 +78,12 @@ class Safe {
     }
   }
 
-  private async check1271SignatureBytes(messageHash: Uint8Array, signature = '0x'): Promise<boolean> {
+  private async check1271SignatureBytes(messageHash: string, signature = '0x'): Promise<boolean> {
     const safeInfo = await this.getInfo();
+    const msgBytes = ethers.utils.arrayify(messageHash);
 
     const encodedIsValidSignatureCall = EIP_1271_BYTES_INTERFACE.encodeFunctionData('isValidSignature', [
-      messageHash,
+      msgBytes,
       signature,
     ]);
 
@@ -110,18 +111,17 @@ class Safe {
   }
 
   async isMessageSigned(message: BytesLike, signature = '0x'): Promise<boolean> {
-    const messageHash = this.calculateMessageHash(message);
+    const messageHash = Safe.calculateMessageHash(message);
     const messageHashSigned = await this.isMessageHashSigned(messageHash, signature);
 
     return messageHashSigned;
   }
 
   async isMessageHashSigned(messageHash: string, signature = '0x'): Promise<boolean> {
-    const checks = [this.check1271Signature, this.check1271SignatureBytes];
+    const checks = [this.check1271Signature.bind(this), this.check1271SignatureBytes.bind(this)];
 
-    const msgBytes = ethers.utils.arrayify(messageHash);
     for (const check of checks) {
-      const isValid = await check(msgBytes, signature);
+      const isValid = await check(messageHash, signature);
       if (isValid) {
         return true;
       }

packages/safe-apps-sdk/src/safe/safe.test.ts

@@ -226,6 +226,41 @@ describe('Safe Apps SDK safe methods', () => {
   });
 
   describe('SDK.safe.isMessageHashSigned', () => {
+    test('Should send call messages to check the message the interface', async () => {
+      const safeInfoSpy = jest.spyOn(sdkInstance.safe, 'getInfo');
+      safeInfoSpy.mockImplementation(
+        (): Promise<SafeInfo> =>
+          Promise.resolve({
+            chainId: 4,
+            safeAddress: '0x9C6FEA0B2eAc5b6D8bBB6C30401D42aA95398190',
+            owners: [],
+            threshold: 1,
+          }),
+      );
+
+      const message = '0x617070726f76652072756770756c6c0000000000000000000000000000000000'; // ethers.utils.formatBytes32String('approve rugpull')
+
+      sdkInstance.safe.isMessageHashSigned(message);
+      await sleep(200);
+      // calling first check1271Signature method
+      expect(postMessageSpy).toHaveBeenCalledWith(
+        expect.objectContaining({
+          method: Methods.rpcCall,
+          params: {
+            call: 'eth_call',
+            params: [
+              {
+                to: '0x9C6FEA0B2eAc5b6D8bBB6C30401D42aA95398190',
+                data: '0x1626ba7e617070726f76652072756770756c6c000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000000',
+              },
+              'latest',
+            ],
+          },
+        }),
+        '*',
+      );
+    });
+
     test('Should return true if check1271Signature return true', async () => {
       const safeInfoSpy = jest.spyOn(sdkInstance.safe, 'getInfo');
       // @ts-expect-error method is private but we are testing it