From 57b8de220eb0ba35e57ecab84c1a6acce0ebc3ea Mon Sep 17 00:00:00 2001 From: Maarten Zuidhoorn Date: Mon, 14 Nov 2022 14:12:37 +0100 Subject: [PATCH] Disallow deriving SIP-6 purpose --- packages/rpc-methods/jest.config.js | 8 ++--- .../src/restricted/getBip32Entropy.test.ts | 30 +++++++++++++++++++ .../rpc-methods/src/restricted/getEntropy.ts | 5 ++-- .../src/manifest/validation.test.ts | 6 ++++ .../snaps-utils/src/manifest/validation.ts | 15 ++++++++++ 5 files changed, 58 insertions(+), 6 deletions(-) diff --git a/packages/rpc-methods/jest.config.js b/packages/rpc-methods/jest.config.js index b0211f082b..bda662d8eb 100644 --- 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 index 4065f0bc9c..597542832f 100644 --- 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 index f972f0f019..0bc0855fa4 100644 --- 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 index 96d082e954..57d04bc041 100644 --- 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 index 0733282b9c..de6004b25a 100644 --- 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; export const Bip32PublicKeyStruct = bip32entropy( @@ -131,6 +143,7 @@ export const Bip32PublicKeyStruct = bip32entropy( compressed: optional(boolean()), }), ); + export type Bip32PublicKey = Infer; 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; export const SnapManifestStruct = object({