From 02dba7b875962095c37a9881d5eed487b0f8350e Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 23 Feb 2021 09:34:45 -0800 Subject: [PATCH 1/4] Fix intermediate multipath bip32 validation --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index 2dcf1d77..85986674 100755 --- a/src/index.js +++ b/src/index.js @@ -38,7 +38,7 @@ const BIP_39_PATH_REGEX = /^bip39:(\w+){1}( \w+){11,23}$/u; * - bip32:44'/bip32:60'/bip32:0'/bip32:0/bip32:0 * - bip39:/bip32:44'/bip32:60'/bip32:0'/bip32:0/bip32:0 */ -const MULTI_PATH_REGEX = /^(bip39:(\w+){1}( \w+){11,23}\/)?(bip32:\d+'?\/){3,4}(bip32:\d+'?)$/u; +const MULTI_PATH_REGEX = /^(bip39:(\w+){1}( \w+){11,23}\/)?(bip32:\d+'?\/){0,4}(bip32:\d+'?)$/u; function validateDeriveKeyParams(pathSegment, parentKey) { // The path segment must be one of the following: From af86951cf45cab82a0b958a31e1359e860ff312a Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 23 Feb 2021 10:55:49 -0800 Subject: [PATCH 2/4] Add input validation test cases --- test/index.js | 78 +++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/test/index.js b/test/index.js index 9b8becec..bcf4d1cf 100755 --- a/test/index.js +++ b/test/index.js @@ -1,22 +1,20 @@ const test = require('tape'); -const { - deriveKeyFromPath, -} = require('../src'); +const { deriveKeyFromPath } = require('../src'); const { bip32: { deriveChildKey: bip32Derive, bip32PathToMultipath, privateKeyToEthAddress, }, - bip39: { - deriveChildKey: bip39Derive, - bip39MnemonicToMultipath, - }, + bip39: { deriveChildKey: bip39Derive, bip39MnemonicToMultipath }, } = require('../src/derivers'); const defaultEthereumPath = `m/44'/60'/0'/0`; -const mnemonic = 'romance hurry grit huge rifle ordinary loud toss sound congress upset twist'; + +const mnemonic = + 'romance hurry grit huge rifle ordinary loud toss sound congress upset twist'; + const expectedAddresses = [ '5df603999c3d5ca2ab828339a9883585b1bce11b', '441c07e32a609afd319ffbb66432b424058bcfe9', @@ -30,25 +28,29 @@ const expectedAddresses = [ '7b99c781cbfff075229314ccbdc7f6d9e8440ad9', ]; -test('ethereum key test - full path', (t) => { +test('deriveKeyPath - full path', (t) => { // generate keys const keys = expectedAddresses.map((_, index) => { const bip32Part = bip32PathToMultipath(`${defaultEthereumPath}/${index}`); const bip39Part = bip39MnemonicToMultipath(mnemonic); const multipath = `${bip39Part}/${bip32Part}`; - t.equal(multipath, `bip39:${mnemonic}/bip32:44'/bip32:60'/bip32:0'/bip32:0/bip32:${index}`, 'matches expected multipath'); + t.strictEqual( + multipath, + `bip39:${mnemonic}/bip32:44'/bip32:60'/bip32:0'/bip32:0/bip32:${index}`, + 'matches expected multipath' + ); return deriveKeyFromPath(multipath); }); // validate addresses keys.forEach((key, index) => { const address = privateKeyToEthAddress(key); - t.equal(address.toString('hex'), expectedAddresses[index]); + t.strictEqual(address.toString('hex'), expectedAddresses[index]); }); t.end(); }); -test('ethereum key test - parent key reuse', (t) => { +test('deriveKeyPath - parent key reuse', (t) => { // generate parent key const bip32Part = bip32PathToMultipath(`${defaultEthereumPath}`); const bip39Part = bip39MnemonicToMultipath(mnemonic); @@ -61,16 +63,62 @@ test('ethereum key test - parent key reuse', (t) => { // validate addresses keys.forEach((key, index) => { const address = privateKeyToEthAddress(key); - t.equal(address.toString('hex'), expectedAddresses[index]); + t.strictEqual(address.toString('hex'), expectedAddresses[index]); }); t.end(); }); +test('deriveKeyPath - input validation', (t) => { + // generate parent key + const bip32Part = bip32PathToMultipath(`${defaultEthereumPath}`); + const bip39Part = bip39MnemonicToMultipath(mnemonic); + const multipath = `${bip39Part}/${bip32Part}`; + const parentKey = deriveKeyFromPath(multipath); + + // Malformed multipaths are disallowed + t.throws(() => { + deriveKeyFromPath(multipath.replace(/bip39/u, `foo`)); + }, /Invalid HD path segment\./u); + t.throws(() => { + deriveKeyFromPath(multipath.replace(/bip32/u, `bar`)); + }, /Invalid HD path segment\./u); + t.throws(() => { + deriveKeyFromPath(multipath.replace(/44'/u, `xyz'`)); + }, /Invalid HD path segment\./u); + t.throws(() => { + deriveKeyFromPath(multipath.replace(/'/u, `"`)); + }, /Invalid HD path segment\./u); + + // Multipaths that start with bip39 segment require _no_ parentKey + t.throws(() => { + deriveKeyFromPath(bip39Part, parentKey); + }, /May not specify parent key and BIP-39 path segment\./u); + + // Multipaths that start with bip32 segment require parentKey + t.throws(() => { + deriveKeyFromPath('bip32:1'); + }, /Must specify parent key/u); + + // parentKey must be a buffer if specified + t.throws( + () => { + deriveKeyFromPath('bip32:1', parentKey.toString('utf8')); + }, + { message: 'Parent key must be a Buffer if specified.' }, + 'should throw expected error' + ); + + t.end(); +}); + test('ethereum key test - direct derive', (t) => { // generate parent key let parentKey = null; - parentKey = bip39Derive(parentKey, `romance hurry grit huge rifle ordinary loud toss sound congress upset twist`); + parentKey = bip39Derive( + parentKey, + `romance hurry grit huge rifle ordinary loud toss sound congress upset twist` + ); parentKey = bip32Derive(parentKey, `44'`); parentKey = bip32Derive(parentKey, `60'`); parentKey = bip32Derive(parentKey, `0'`); @@ -81,7 +129,7 @@ test('ethereum key test - direct derive', (t) => { // validate addresses keys.forEach((key, index) => { const address = privateKeyToEthAddress(key); - t.equal(address.toString('hex'), expectedAddresses[index]); + t.strictEqual(address.toString('hex'), expectedAddresses[index]); }); t.end(); From a1186b656562e15d9209dce5fa88ad4eb1e5d6de Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 23 Feb 2021 11:00:58 -0800 Subject: [PATCH 3/4] fixup! Add input validation test cases --- test/index.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/index.js b/test/index.js index bcf4d1cf..84e9b6b5 100755 --- a/test/index.js +++ b/test/index.js @@ -37,7 +37,7 @@ test('deriveKeyPath - full path', (t) => { t.strictEqual( multipath, `bip39:${mnemonic}/bip32:44'/bip32:60'/bip32:0'/bip32:0/bip32:${index}`, - 'matches expected multipath' + 'matches expected multipath', ); return deriveKeyFromPath(multipath); }); @@ -106,7 +106,7 @@ test('deriveKeyPath - input validation', (t) => { deriveKeyFromPath('bip32:1', parentKey.toString('utf8')); }, { message: 'Parent key must be a Buffer if specified.' }, - 'should throw expected error' + 'should throw expected error', ); t.end(); @@ -117,7 +117,7 @@ test('ethereum key test - direct derive', (t) => { let parentKey = null; parentKey = bip39Derive( parentKey, - `romance hurry grit huge rifle ordinary loud toss sound congress upset twist` + `romance hurry grit huge rifle ordinary loud toss sound congress upset twist`, ); parentKey = bip32Derive(parentKey, `44'`); parentKey = bip32Derive(parentKey, `60'`); From bd0d9eabfe623ce1e879b8cbc66c7db83851d4ef Mon Sep 17 00:00:00 2001 From: Erik Marks Date: Tue, 23 Feb 2021 13:47:38 -0800 Subject: [PATCH 4/4] Update test title --- test/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/index.js b/test/index.js index 84e9b6b5..cbb7bbd4 100755 --- a/test/index.js +++ b/test/index.js @@ -112,7 +112,7 @@ test('deriveKeyPath - input validation', (t) => { t.end(); }); -test('ethereum key test - direct derive', (t) => { +test('bip32Derive', (t) => { // generate parent key let parentKey = null; parentKey = bip39Derive(