Skip to content

Commit

Permalink
Bug: Missing context in signature checks (#213)
Browse files Browse the repository at this point in the history
* bind signature checks

* add test checking isMessageHashSigned sends a message to the interface

* fix types

* include dist files

* make calculateMessageHash static

* make calculateMessageHash static

* make calculateMessageHash a util function

* include dist files
  • Loading branch information
mmv08 authored Sep 6, 2021
1 parent 35c8b88 commit 5286af8
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 60 deletions.
5 changes: 5 additions & 0 deletions .changeset/cool-ladybugs-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@gnosis.pm/safe-apps-sdk': patch
---

fix missing context on signature checks
2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/dist/package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
1 change: 1 addition & 0 deletions packages/safe-apps-sdk/dist/src/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ export * from './sdk';
export * from './types';
export * from './communication/methods';
export * from './communication/messageFormatter';
export { calculateMessageHash } from './safe/signatures';
export { getSDKVersion } from './utils';
4 changes: 3 additions & 1 deletion packages/safe-apps-sdk/dist/src/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/dist/src/index.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/dist/src/safe/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@ declare class Safe {
constructor(communicator: Communicator);
getInfo(): Promise<SafeInfo>;
experimental_getBalances({ currency }?: GetBalanceParams): Promise<SafeBalances>;
calculateMessageHash(message: BytesLike): string;
private check1271Signature;
private check1271SignatureBytes;
isMessageSigned(message: BytesLike, signature?: string): Promise<boolean>;
isMessageHashSigned(messageHash: string, signature?: string): Promise<boolean>;
}
export { Safe };
32 changes: 9 additions & 23 deletions packages/safe-apps-sdk/dist/src/safe/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/dist/src/safe/index.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 3 additions & 1 deletion packages/safe-apps-sdk/dist/src/safe/signatures.d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
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;
export { EIP_1271_INTERFACE, EIP_1271_BYTES_INTERFACE, MAGIC_VALUE, MAGIC_VALUE_BYTES };
declare const calculateMessageHash: (message: BytesLike) => string;
export { EIP_1271_INTERFACE, EIP_1271_BYTES_INTERFACE, MAGIC_VALUE, MAGIC_VALUE_BYTES, calculateMessageHash };
9 changes: 8 additions & 1 deletion packages/safe-apps-sdk/dist/src/safe/signatures.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/dist/src/safe/signatures.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/safe-apps-sdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
1 change: 1 addition & 0 deletions packages/safe-apps-sdk/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ export * from './sdk';
export * from './types';
export * from './communication/methods';
export * from './communication/messageFormatter';
export { calculateMessageHash } from './safe/signatures';
export { getSDKVersion } from './utils';
30 changes: 14 additions & 16 deletions packages/safe-apps-sdk/src/safe/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { ethers } from 'ethers';
import { EIP_1271_INTERFACE, EIP_1271_BYTES_INTERFACE, MAGIC_VALUE_BYTES, MAGIC_VALUE } from './signatures';
import {
EIP_1271_INTERFACE,
EIP_1271_BYTES_INTERFACE,
MAGIC_VALUE_BYTES,
MAGIC_VALUE,
calculateMessageHash,
} from './signatures';
import { Methods } from '../communication/methods';
import { RPC_CALLS } from '../eth/constants';
import {
Expand Down Expand Up @@ -40,15 +46,7 @@ class Safe {
return response.data;
}

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', [
Expand Down Expand Up @@ -78,11 +76,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,
]);

Expand Down Expand Up @@ -110,18 +109,17 @@ class Safe {
}

async isMessageSigned(message: BytesLike, signature = '0x'): Promise<boolean> {
const messageHash = this.calculateMessageHash(message);
const messageHash = 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;
}
Expand Down
Loading

0 comments on commit 5286af8

Please sign in to comment.