Fix BIP-32 path validation (#1506)

* added audit bip32 validation fix * Use key-tree util for BIP32 validation * Update shasums --------- Co-authored-by: Frederik Bolding <[email protected]>
MetaMask · Jun 21, 2023 · 43e2801 · 43e2801
1 parent 6d3799b
commit 43e2801
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 36 deletions.
diff --git a/packages/examples/examples/signer/packages/bitcoin/package.json b/packages/examples/examples/signer/packages/bitcoin/package.json
@@ -11,7 +11,7 @@
     "start": "mm-snap serve"
   },
   "dependencies": {
-    "@metamask/key-tree": "^7.0.0",
+    "@metamask/key-tree": "^7.1.1",
     "@metamask/snaps-types": "workspace:^",
     "@metamask/utils": "^6.0.1",
     "@noble/secp256k1": "^1.7.1",

diff --git a/packages/examples/examples/signer/packages/core/package.json b/packages/examples/examples/signer/packages/core/package.json
@@ -15,7 +15,7 @@
     "async-mutex": "^0.4.0"
   },
   "devDependencies": {
-    "@metamask/key-tree": "^7.0.0",
+    "@metamask/key-tree": "^7.1.1",
     "@metamask/snaps-cli": "workspace:^",
     "@metamask/snaps-types": "workspace:^"
   },

diff --git a/packages/examples/examples/signer/packages/core/snap.manifest.json b/packages/examples/examples/signer/packages/core/snap.manifest.json
@@ -7,7 +7,7 @@
     "url": "https://github.com/MetaMask/snaps-monorepo.git"
   },
   "source": {
-    "shasum": "gnA/2nadebS8BdjfHgPA1cxfQVdP9y89XSEY6i4qAHc=",
+    "shasum": "XEeIJvHwYV6utwGRkFLofazQRlGAXjmrH8lIKcgp3ok=",
     "location": {
       "npm": {
         "filePath": "dist/bundle.js",

diff --git a/packages/examples/examples/signer/packages/ethereum/package.json b/packages/examples/examples/signer/packages/ethereum/package.json
@@ -12,7 +12,7 @@
   },
   "dependencies": {
     "@ethereumjs/tx": "^3.5.2",
-    "@metamask/key-tree": "^7.0.0",
+    "@metamask/key-tree": "^7.1.1",
     "@metamask/snaps-types": "workspace:^",
     "@metamask/utils": "^6.0.1",
     "buffer": "^6.0.3"

diff --git a/packages/rpc-methods/package.json b/packages/rpc-methods/package.json
@@ -29,7 +29,7 @@
   },
   "dependencies": {
     "@metamask/browser-passworder": "^4.0.2",
-    "@metamask/key-tree": "^7.0.0",
+    "@metamask/key-tree": "^7.1.1",
     "@metamask/permission-controller": "^4.0.0",
     "@metamask/snaps-ui": "workspace:^",
     "@metamask/snaps-utils": "workspace:^",

diff --git a/packages/snaps-utils/package.json b/packages/snaps-utils/package.json
@@ -56,6 +56,7 @@
     "@babel/core": "^7.18.6",
     "@babel/types": "^7.18.7",
     "@metamask/base-controller": "^3.0.0",
+    "@metamask/key-tree": "^7.1.1",
     "@metamask/permission-controller": "^4.0.0",
     "@metamask/providers": "^10.2.1",
     "@metamask/snaps-registry": "^1.2.1",
@@ -82,7 +83,6 @@
     "@metamask/eslint-config-jest": "^11.0.0",
     "@metamask/eslint-config-nodejs": "^11.0.1",
     "@metamask/eslint-config-typescript": "^11.0.0",
-    "@metamask/key-tree": "^7.0.0",
     "@metamask/post-message-stream": "^6.1.1",
     "@types/jest": "^27.5.1",
     "@types/mocha": "^10.0.1",

diff --git a/packages/snaps-utils/src/manifest/validation.test.ts b/packages/snaps-utils/src/manifest/validation.test.ts
@@ -52,14 +52,17 @@ describe('Bip32PathStruct', () => {
     );
   });
 
-  it.each(["m/0'/123/asd", 'm/0"/123', 'm/1/2/3/_', "m/1'/2'/3'/-1"])(
-    'requires numbers or hardened numbers',
-    (path) => {
-      expect(() => assert(path.split('/'), Bip32PathStruct)).toThrow(
-        'Path must be a valid BIP-32 derivation path array.',
-      );
-    },
-  );
+  it.each([
+    "m/0'/123/asd",
+    'm/0"/123',
+    'm/1/2/3/_',
+    "m/1'/2'/3'/-1",
+    "m/1'/2147483648'",
+  ])('requires numbers or hardened numbers per BIP32', (path) => {
+    expect(() => assert(path.split('/'), Bip32PathStruct)).toThrow(
+      'Path must be a valid BIP-32 derivation path array.',
+    );
+  });
 
   it('throws for forbidden purposes', () => {
     expect(() => assert(['m', "1399742832'", '0'], Bip32PathStruct)).toThrow(

diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts
@@ -1,3 +1,4 @@
+import { isValidBIP32PathSegment } from '@metamask/key-tree';
 import { assertStruct, ChecksumStruct, VersionStruct } from '@metamask/utils';
 import {
   array,
@@ -40,11 +41,10 @@ const FORBIDDEN_PATHS: string[][] = FORBIDDEN_COIN_TYPES.map((coinType) => [
   `${coinType}'`,
 ]);
 
-const BIP32_INDEX_REGEX = /^\d+'?$/u;
 export const Bip32PathStruct = refine(
   array(string()),
   'BIP-32 path',
-  (path) => {
+  (path: string[]) => {
     if (path.length === 0) {
       return 'Path must be a non-empty BIP-32 derivation path array';
     }
@@ -57,7 +57,7 @@ export const Bip32PathStruct = refine(
       return 'Paths must have a length of at least three.';
     }
 
-    if (path.slice(1).some((part) => !BIP32_INDEX_REGEX.test(part))) {
+    if (path.slice(1).some((part) => !isValidBIP32PathSegment(part))) {
       return 'Path must be a valid BIP-32 derivation path array.';
     }
 

diff --git a/yarn.lock b/yarn.lock
@@ -3050,17 +3050,17 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/key-tree@npm:^7.0.0":
-  version: 7.0.0
-  resolution: "@metamask/key-tree@npm:7.0.0"
+"@metamask/key-tree@npm:^7.1.1":
+  version: 7.1.1
+  resolution: "@metamask/key-tree@npm:7.1.1"
   dependencies:
     "@metamask/scure-bip39": ^2.1.0
-    "@metamask/utils": ^3.3.0
+    "@metamask/utils": ^6.0.1
     "@noble/ed25519": ^1.6.0
     "@noble/hashes": ^1.0.0
     "@noble/secp256k1": ^1.5.5
     "@scure/base": ^1.0.0
-  checksum: e3b8371ba41766e1936ff0b0b34c46b7a1297e51fe177246ae6446c4f15a6601a6ed4fc5d01d0594e8b1e9b1682096900f7ab4e7e15df94041ec025116a635b9
+  checksum: ddab7917e1214c51508f821a680f8e8819a8c866fd91244e0debf28464331dfcbf9a357e01716fc593cf00a8c71f95fd45ed4ff686ee6abc4c4061aed629537c
   languageName: node
   linkType: hard
 
@@ -3172,7 +3172,7 @@ __metadata:
     "@metamask/eslint-config-jest": ^11.0.0
     "@metamask/eslint-config-nodejs": ^11.0.1
     "@metamask/eslint-config-typescript": ^11.0.0
-    "@metamask/key-tree": ^7.0.0
+    "@metamask/key-tree": ^7.1.1
     "@metamask/permission-controller": ^4.0.0
     "@metamask/snaps-ui": "workspace:^"
     "@metamask/snaps-utils": "workspace:^"
@@ -3614,7 +3614,7 @@ __metadata:
     "@metamask/eslint-config-jest": ^11.0.0
     "@metamask/eslint-config-nodejs": ^11.0.1
     "@metamask/eslint-config-typescript": ^11.0.0
-    "@metamask/key-tree": ^7.0.0
+    "@metamask/key-tree": ^7.1.1
     "@metamask/permission-controller": ^4.0.0
     "@metamask/post-message-stream": ^6.1.1
     "@metamask/providers": ^10.2.1
@@ -3724,7 +3724,7 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@metamask/utils@npm:^3.3.0, @metamask/utils@npm:^3.4.1":
+"@metamask/utils@npm:^3.4.1":
   version: 3.6.0
   resolution: "@metamask/utils@npm:3.6.0"
   dependencies:
@@ -4581,20 +4581,20 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@types/node@npm:*, @types/node@npm:^18.0.0, @types/node@npm:^18.14.0":
-  version: 18.14.2
-  resolution: "@types/node@npm:18.14.2"
-  checksum: 53c07e721f6ae33de71306f6a0b75dae6066a4f55bd5484c93bd59ff25f0c5f004ceafeef509a4d0cb9e24a247efc34d50489bcc1b05a53ecc68e2fc088e65cb
-  languageName: node
-  linkType: hard
-
-"@types/node@npm:^20.3.1":
+"@types/node@npm:*, @types/node@npm:^20.3.1":
   version: 20.3.1
   resolution: "@types/node@npm:20.3.1"
   checksum: 63a393ab6d947be17320817b35d7277ef03728e231558166ed07ee30b09fd7c08861be4d746f10fdc63ca7912e8cd023939d4eab887ff6580ff704ff24ed810c
   languageName: node
   linkType: hard
 
+"@types/node@npm:^18.0.0, @types/node@npm:^18.14.0":
+  version: 18.14.2
+  resolution: "@types/node@npm:18.14.2"
+  checksum: 53c07e721f6ae33de71306f6a0b75dae6066a4f55bd5484c93bd59ff25f0c5f004ceafeef509a4d0cb9e24a247efc34d50489bcc1b05a53ecc68e2fc088e65cb
+  languageName: node
+  linkType: hard
+
 "@types/normalize-package-data@npm:^2.4.1":
   version: 2.4.1
   resolution: "@types/normalize-package-data@npm:2.4.1"
@@ -15944,7 +15944,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "signer-bitcoin@workspace:packages/examples/examples/signer/packages/bitcoin"
   dependencies:
-    "@metamask/key-tree": ^7.0.0
+    "@metamask/key-tree": ^7.1.1
     "@metamask/snaps-cli": "workspace:^"
     "@metamask/snaps-types": "workspace:^"
     "@metamask/utils": ^6.0.1
@@ -15958,7 +15958,7 @@ __metadata:
   version: 0.0.0-use.local
   resolution: "signer-core@workspace:packages/examples/examples/signer/packages/core"
   dependencies:
-    "@metamask/key-tree": ^7.0.0
+    "@metamask/key-tree": ^7.1.1
     "@metamask/snaps-cli": "workspace:^"
     "@metamask/snaps-types": "workspace:^"
     "@metamask/utils": ^6.0.1
@@ -15971,7 +15971,7 @@ __metadata:
   resolution: "signer-ethereum@workspace:packages/examples/examples/signer/packages/ethereum"
   dependencies:
     "@ethereumjs/tx": ^3.5.2
-    "@metamask/key-tree": ^7.0.0
+    "@metamask/key-tree": ^7.1.1
     "@metamask/snaps-cli": "workspace:^"
     "@metamask/snaps-types": "workspace:^"
     "@metamask/utils": ^6.0.1