Disallow deriving SIP-6 purpose

MetaMask · Nov 14, 2022 · 57b8de2 · 57b8de2
1 parent 9dcd86c
commit 57b8de2
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 6 deletions.
diff --git a/packages/rpc-methods/jest.config.js b/packages/rpc-methods/jest.config.js
@@ -5,10 +5,10 @@ module.exports = deepmerge(baseConfig, {
   coveragePathIgnorePatterns: ['./src/index.ts'],
   coverageThreshold: {
     global: {
-      branches: 87.01,
-      functions: 87.03,
-      lines: 78.96,
-      statements: 78.96,
+      branches: 87.5,
+      functions: 88.13,
+      lines: 80.89,
+      statements: 80.89,
     },
   },
   testTimeout: 2500,

diff --git a/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts b/packages/rpc-methods/src/restricted/getBip32Entropy.test.ts
@@ -6,6 +6,7 @@ import {
   getBip32EntropyImplementation,
   validateCaveatPaths,
 } from './getBip32Entropy';
+import { SIP_6_MAGIC_VALUE } from './getEntropy';
 
 const TEST_SECRET_RECOVERY_PHRASE =
   'test test test test test test test test test test test ball';
@@ -196,6 +197,22 @@ describe('getBip32EntropyCaveatSpecifications', () => {
         'The requested path is not permitted. Allowed paths must be specified in the snap manifest.',
       );
     });
+
+    it('throws if the purpose is not allowed', async () => {
+      const fn = jest.fn().mockImplementation(() => 'foo');
+
+      await expect(
+        getBip32EntropyCaveatSpecifications[
+          SnapCaveatType.PermittedDerivationPaths
+        ].decorator(fn, {
+          type: SnapCaveatType.PermittedDerivationPaths,
+          value: [params],
+          // @ts-expect-error Missing other required properties.
+        })({ params: { ...params, path: ['m', SIP_6_MAGIC_VALUE, "0'"] } }),
+      ).rejects.toThrow(
+        'Invalid BIP-32 entropy path definition: At path: path -- The purpose "1399742832\'" is not allowed for entropy derivation.',
+      );
+    });
   });
 
   describe('validator', () => {
@@ -209,6 +226,19 @@ describe('getBip32EntropyCaveatSpecifications', () => {
         }),
       ).toThrow('At path: value.0.path -- Path must start with "m".');
     });
+
+    it('throws if the caveat values contain forbidden paths', () => {
+      expect(() =>
+        getBip32EntropyCaveatSpecifications[
+          SnapCaveatType.PermittedDerivationPaths
+        ].validator?.({
+          type: SnapCaveatType.PermittedDerivationPaths,
+          value: [{ path: ['m', SIP_6_MAGIC_VALUE, "0'"], curve: 'secp256k1' }],
+        }),
+      ).toThrow(
+        'Invalid BIP-32 entropy caveat: At path: value.0.path -- The purpose "1399742832\'" is not allowed for entropy derivation.',
+      );
+    });
   });
 });
 

diff --git a/packages/rpc-methods/src/restricted/getEntropy.ts b/packages/rpc-methods/src/restricted/getEntropy.ts
@@ -18,7 +18,8 @@ import {
 } from '@metamask/controllers';
 import { keccak_256 as keccak256 } from '@noble/hashes/sha3';
 
-const MAGIC_VALUE = 0xd36e6170;
+// 0xd36e6170 - 0x80000000
+export const SIP_6_MAGIC_VALUE = `1399742832'` as `${number}'`;
 const HARDENED_VALUE = 0x80000000;
 
 const targetKey = 'snap_getEntropy';
@@ -139,7 +140,7 @@ export async function deriveEntropy(
   const { privateKey } = await SLIP10Node.fromDerivationPath({
     derivationPath: [
       `bip39:${mnemonicPhrase}`,
-      `bip32:${MAGIC_VALUE - HARDENED_VALUE}'`,
+      `bip32:${SIP_6_MAGIC_VALUE}`,
       ...computedDerivationPath,
     ],
     curve: 'secp256k1',

diff --git a/packages/snaps-utils/src/manifest/validation.test.ts b/packages/snaps-utils/src/manifest/validation.test.ts
@@ -121,6 +121,12 @@ describe('Bip32PathStruct', () => {
       );
     },
   );
+
+  it('throws for forbidden paths', () => {
+    expect(() => assert(['m', "1399742832'", '0'], Bip32PathStruct)).toThrow(
+      'The purpose "1399742832\'" is not allowed for entropy derivation.',
+    );
+  });
 });
 
 describe('Bip32EntropyStruct', () => {

diff --git a/packages/snaps-utils/src/manifest/validation.ts b/packages/snaps-utils/src/manifest/validation.ts
@@ -21,6 +21,13 @@ import { CronjobSpecificationArrayStruct } from '../cronjob';
 import { NamespacesStruct } from '../namespace';
 import { NameStruct, NpmSnapFileNames, VersionStruct } from '../types';
 
+// BIP-43 purposes that cannot be used for entropy derivation. These are in the
+// string form, ending with `'`.
+const FORBIDDEN_PURPOSES: string[] = [
+  // SIP-6 magic value.
+  `1399742832'`,
+];
+
 export type Base64Opts = {
   /**
    * Is the `=` padding at the end required or not.
@@ -97,6 +104,10 @@ export const Bip32PathStruct = refine(
       return 'Path must be a valid BIP-32 derivation path array.';
     }
 
+    if (FORBIDDEN_PURPOSES.includes(path[1])) {
+      return `The purpose "${path[1]}" is not allowed for entropy derivation.`;
+    }
+
     return true;
   },
 );
@@ -122,6 +133,7 @@ export const Bip32EntropyStruct = bip32entropy(
     curve: enums(['ed25519', 'secp256k1']),
   }),
 );
+
 export type Bip32Entropy = Infer<typeof Bip32EntropyStruct>;
 
 export const Bip32PublicKeyStruct = bip32entropy(
@@ -131,6 +143,7 @@ export const Bip32PublicKeyStruct = bip32entropy(
     compressed: optional(boolean()),
   }),
 );
+
 export type Bip32PublicKey = Infer<typeof Bip32PublicKeyStruct>;
 
 const PermissionsStruct = type({
@@ -154,12 +167,14 @@ const PermissionsStruct = type({
       Infinity,
     ),
   ),
+  snap_getEntropy: optional(object({})),
   'endowment:keyring': optional(
     object({
       namespaces: NamespacesStruct,
     }),
   ),
 });
+
 export type SnapPermissions = Infer<typeof PermissionsStruct>;
 
 export const SnapManifestStruct = object({