From 2aa5c65945f80d0400624e218fff3a99d7bf04cc Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Fri, 2 Jan 2015 16:46:37 -0300 Subject: [PATCH 1/6] Add validations to derivation path --- lib/hdprivatekey.js | 24 ++++++++++++++++++++++++ lib/hdpublickey.js | 39 ++++++++++++++++++++++++++++++--------- test/hdprivatekey.js | 18 ++++++++++++++++++ test/hdpublickey.js | 20 +++++++++++++++++++- 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/lib/hdprivatekey.js b/lib/hdprivatekey.js index ad53e1c..8d2d222 100644 --- a/lib/hdprivatekey.js +++ b/lib/hdprivatekey.js @@ -62,6 +62,22 @@ function HDPrivateKey(arg) { } } +/** + * Verifies that a given path is valid. + * + * @param {string|number} arg + * @param {boolean?} hardened + * @return {boolean} + */ +HDPrivateKey.prototype.isValidPath = function(arg, hardened) { + try { + this.derive(arg, hardened); + return true; + } catch (err) { + return false; + } +}; + /** * Get a derivated child based on a string or number. * @@ -101,9 +117,15 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) { if (index >= HDPrivateKey.Hardened) { hardened = true; } + if (index < HDPrivateKey.Hardened && hardened) { index += HDPrivateKey.Hardened; } + + if (index < 0 || index >= HDPrivateKey.MaxIndex) { + throw new hdErrors.InvalidPath(index); + } + var cached = HDKeyCache.get(this.xprivkey, index, hardened); if (cached) { return cached; @@ -443,6 +465,8 @@ HDPrivateKey.DefaultFingerprint = 0; HDPrivateKey.DefaultChildIndex = 0; HDPrivateKey.DefaultNetwork = Network.livenet; HDPrivateKey.Hardened = 0x80000000; +HDPrivateKey.MaxIndex = 2 * HDPrivateKey.Hardened; + HDPrivateKey.RootElementAlias = ['m', 'M', 'm\'', 'M\'']; HDPrivateKey.VersionSize = 4; diff --git a/lib/hdpublickey.js b/lib/hdpublickey.js index 158aa05..979faf9 100644 --- a/lib/hdpublickey.js +++ b/lib/hdpublickey.js @@ -65,6 +65,22 @@ function HDPublicKey(arg) { } } + +/** + * Verifies that a given path is valid. + * + * @param {string|number} arg + * @return {boolean} + */ +HDPublicKey.prototype.isValidPath = function(arg) { + try { + this.derive(arg); + return true; + } catch (err) { + return false; + } +}; + /** * Get a derivated child based on a string or number. * @@ -86,11 +102,10 @@ function HDPublicKey(arg) { * ``` * * @param {string|number} arg - * @param {boolean?} hardened */ -HDPublicKey.prototype.derive = function (arg, hardened) { +HDPublicKey.prototype.derive = function (arg) { if (_.isNumber(arg)) { - return this._deriveWithNumber(arg, hardened); + return this._deriveWithNumber(arg); } else if (_.isString(arg)) { return this._deriveFromString(arg); } else { @@ -98,11 +113,14 @@ HDPublicKey.prototype.derive = function (arg, hardened) { } }; -HDPublicKey.prototype._deriveWithNumber = function (index, hardened) { - if (hardened || index >= HDPublicKey.Hardened) { +HDPublicKey.prototype._deriveWithNumber = function (index) { + if (index >= HDPublicKey.Hardened) { throw new hdErrors.InvalidIndexCantDeriveHardened(); } - var cached = HDKeyCache.get(this.xpubkey, index, hardened); + if (index < 0) { + throw new hdErrors.InvalidPath(index); + } + var cached = HDKeyCache.get(this.xpubkey, index, false); if (cached) { return cached; } @@ -123,12 +141,16 @@ HDPublicKey.prototype._deriveWithNumber = function (index, hardened) { chainCode: chainCode, publicKey: publicKey }); - HDKeyCache.set(this.xpubkey, index, hardened, derived); + HDKeyCache.set(this.xpubkey, index, false, derived); return derived; }; HDPublicKey.prototype._deriveFromString = function (path) { /* jshint maxcomplexity: 8 */ + if (_.contains(path, "'")) { + throw new hdErrors.InvalidIndexCantDeriveHardened(); + } + var steps = path.split('/'); // Special cases: @@ -143,8 +165,7 @@ HDPublicKey.prototype._deriveFromString = function (path) { var result = this; for (var step in steps) { var index = parseInt(steps[step]); - var hardened = steps[step] !== index.toString(); - result = result._deriveWithNumber(index, hardened); + result = result._deriveWithNumber(index); } return result; }; diff --git a/test/hdprivatekey.js b/test/hdprivatekey.js index 6ba3b4d..aec7563 100644 --- a/test/hdprivatekey.js +++ b/test/hdprivatekey.js @@ -190,6 +190,24 @@ describe('HDPrivate key interface', function() { derivedByNumber.xprivkey.should.equal(derivedByString.xprivkey); }); + it('validates correct paths', function() { + var privateKey = new HDPrivateKey(xprivkey); + var valid = privateKey.isValidPath('m/0\'/1/2\''); + valid.should.equal(true); + + var valid = privateKey.isValidPath(123, true); + valid.should.equal(true); + }); + + it('validates illigal paths', function() { + var privateKey = new HDPrivateKey(xprivkey); + var valid = privateKey.isValidPath('m/-1/12'); + valid.should.equal(false); + + var valid = privateKey.isValidPath(HDPrivateKey.MaxHardened); + valid.should.equal(false); + }); + describe('conversion to plain object/json', function() { var plainObject = { 'network':'livenet', diff --git a/test/hdpublickey.js b/test/hdpublickey.js index 721fca8..be25b7b 100644 --- a/test/hdpublickey.js +++ b/test/hdpublickey.js @@ -216,10 +216,28 @@ describe('HDPublicKey interface', function() { it('can\'t derive hardened keys', function() { expectFail(function() { - return new HDPublicKey(xpubkey).derive(HDPublicKey.Hardened + 1); + return new HDPublicKey(xpubkey).derive(HDPublicKey.Hardened); }, hdErrors.InvalidDerivationArgument); }); + it('validates correct paths', function() { + var publicKey = new HDPublicKey(xpubkey); + var valid = publicKey.isValidPath('m/123/12'); + valid.should.equal(true); + + var valid = publicKey.isValidPath(123, true); + valid.should.equal(true); + }); + + it('validates illigal paths', function() { + var publicKey = new HDPublicKey(xpubkey); + var valid = publicKey.isValidPath('m/-1/12'); + valid.should.equal(false); + + var valid = publicKey.isValidPath(HDPublicKey.Hardened); + valid.should.equal(false); + }); + it('should use the cache', function() { var pubkey = new HDPublicKey(xpubkey); var derived1 = pubkey.derive(0); From bb3064d336cbe8d429b01e187443dbd17756ffbd Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Fri, 2 Jan 2015 18:03:40 -0300 Subject: [PATCH 2/6] fix typo --- test/hdprivatekey.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hdprivatekey.js b/test/hdprivatekey.js index aec7563..4e7688b 100644 --- a/test/hdprivatekey.js +++ b/test/hdprivatekey.js @@ -199,7 +199,7 @@ describe('HDPrivate key interface', function() { valid.should.equal(true); }); - it('validates illigal paths', function() { + it('validates illegal paths', function() { var privateKey = new HDPrivateKey(xprivkey); var valid = privateKey.isValidPath('m/-1/12'); valid.should.equal(false); From 884cae7349221e9388e129707cc9bdaa1bc352ca Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Tue, 6 Jan 2015 10:33:31 -0300 Subject: [PATCH 3/6] Fix another typo --- test/hdpublickey.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/hdpublickey.js b/test/hdpublickey.js index be25b7b..dd73aca 100644 --- a/test/hdpublickey.js +++ b/test/hdpublickey.js @@ -229,7 +229,7 @@ describe('HDPublicKey interface', function() { valid.should.equal(true); }); - it('validates illigal paths', function() { + it('validates illegal paths', function() { var publicKey = new HDPublicKey(xpubkey); var valid = publicKey.isValidPath('m/-1/12'); valid.should.equal(false); From f78ebeb46c91fefa87dab9c4ac126d3648b597a5 Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Tue, 6 Jan 2015 11:51:58 -0300 Subject: [PATCH 4/6] Refactor HDPrivateKey path validation --- lib/hdprivatekey.js | 85 ++++++++++++++++++++++++++++---------------- test/hdprivatekey.js | 74 +++++++++++++++++++++++++++++++------- 2 files changed, 116 insertions(+), 43 deletions(-) diff --git a/lib/hdprivatekey.js b/lib/hdprivatekey.js index 8d2d222..b8409f8 100644 --- a/lib/hdprivatekey.js +++ b/lib/hdprivatekey.js @@ -69,15 +69,51 @@ function HDPrivateKey(arg) { * @param {boolean?} hardened * @return {boolean} */ -HDPrivateKey.prototype.isValidPath = function(arg, hardened) { - try { - this.derive(arg, hardened); - return true; - } catch (err) { - return false; +HDPrivateKey.isValidPath = function(arg, hardened) { + if (_.isString(arg)) { + var indexes = HDPrivateKey._getDerivationIndexes(arg); + return indexes !== null && _.all(indexes, HDPrivateKey.isValidPath); } + + if (_.isNumber(arg)) { + if (arg < HDPrivateKey.Hardened && hardened === true) { + arg += HDPrivateKey.Hardened; + } + return arg >= 0 && arg < HDPrivateKey.MaxIndex; + } + + return false; }; +/** + * Internal function that splits a string path into a derivation index array. + * It will return null if the string path is malformed. + * It does not validates if a indexes are in bounds. + * + * @param {string} path + * @return {Array} + */ +HDPrivateKey._getDerivationIndexes = function(path) { + var steps = path.split('/'); + + // Special cases: + if (_.contains(HDPrivateKey.RootElementAlias, path)) { + return []; + } + + if (!_.contains(HDPrivateKey.RootElementAlias, steps[0])) { + return null; + } + + var indexes = steps.slice(1).map(function(step) { + var index = parseInt(step); + index += step != index.toString() ? HDPrivateKey.Hardened : 0; + return index; + }); + + return _.any(indexes, isNaN) ? null : indexes; +} + /** * Get a derivated child based on a string or number. * @@ -114,18 +150,15 @@ HDPrivateKey.prototype.derive = function(arg, hardened) { HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) { /* jshint maxstatements: 20 */ /* jshint maxcomplexity: 10 */ - if (index >= HDPrivateKey.Hardened) { - hardened = true; - } - - if (index < HDPrivateKey.Hardened && hardened) { - index += HDPrivateKey.Hardened; - } - - if (index < 0 || index >= HDPrivateKey.MaxIndex) { + if (!HDPrivateKey.isValidPath(index, hardened)) { throw new hdErrors.InvalidPath(index); } + hardened = index >= HDPrivateKey.Hardened ? true : hardened; + if (index < HDPrivateKey.Hardened && hardened === true) { + index += HDPrivateKey.Hardened; + } + var cached = HDKeyCache.get(this.xprivkey, index, hardened); if (cached) { return cached; @@ -157,24 +190,16 @@ HDPrivateKey.prototype._deriveWithNumber = function(index, hardened) { }; HDPrivateKey.prototype._deriveFromString = function(path) { - var steps = path.split('/'); - - // Special cases: - if (_.contains(HDPrivateKey.RootElementAlias, path)) { - return this; - } - if (!_.contains(HDPrivateKey.RootElementAlias, steps[0])) { + if (!HDPrivateKey.isValidPath(path)) { throw new hdErrors.InvalidPath(path); } - steps = steps.slice(1); - var result = this; - for (var step in steps) { - var index = parseInt(steps[step]); - var hardened = steps[step] !== index.toString(); - result = result._deriveWithNumber(index, hardened); - } - return result; + var indexes = HDPrivateKey._getDerivationIndexes(path); + var derived = indexes.reduce(function(prev, index) { + return prev._deriveWithNumber(index); + }, this); + + return derived; }; /** diff --git a/test/hdprivatekey.js b/test/hdprivatekey.js index 4e7688b..6191743 100644 --- a/test/hdprivatekey.js +++ b/test/hdprivatekey.js @@ -190,22 +190,70 @@ describe('HDPrivate key interface', function() { derivedByNumber.xprivkey.should.equal(derivedByString.xprivkey); }); - it('validates correct paths', function() { - var privateKey = new HDPrivateKey(xprivkey); - var valid = privateKey.isValidPath('m/0\'/1/2\''); - valid.should.equal(true); + describe('validates paths', function() { + it('validates correct paths', function() { + var valid; - var valid = privateKey.isValidPath(123, true); - valid.should.equal(true); - }); + valid = HDPrivateKey.isValidPath("m/0'/1/2'"); + valid.should.equal(true); - it('validates illegal paths', function() { - var privateKey = new HDPrivateKey(xprivkey); - var valid = privateKey.isValidPath('m/-1/12'); - valid.should.equal(false); + valid = HDPrivateKey.isValidPath('m'); + valid.should.equal(true); - var valid = privateKey.isValidPath(HDPrivateKey.MaxHardened); - valid.should.equal(false); + valid = HDPrivateKey.isValidPath(123, true); + valid.should.equal(true); + + valid = HDPrivateKey.isValidPath(123); + valid.should.equal(true); + + valid = HDPrivateKey.isValidPath(HDPrivateKey.Hardened + 123); + valid.should.equal(true); + + valid = HDPrivateKey.isValidPath(HDPrivateKey.Hardened + 123, true); + valid.should.equal(true); + }); + + it('rejects illegal paths', function() { + var valid; + + valid = HDPrivateKey.isValidPath('m/-1/12'); + valid.should.equal(false); + + valid = HDPrivateKey.isValidPath('bad path'); + valid.should.equal(false); + + valid = HDPrivateKey.isValidPath('K'); + valid.should.equal(false); + + valid = HDPrivateKey.isValidPath('m/'); + valid.should.equal(false); + + valid = HDPrivateKey.isValidPath(HDPrivateKey.MaxHardened); + valid.should.equal(false); + }); + + it('generates deriving indexes correctly', function() { + var indexes; + + indexes = HDPrivateKey._getDerivationIndexes('m/-1/12'); + indexes.should.eql([-1, 12]); + + indexes = HDPrivateKey._getDerivationIndexes("m/0/12/12'"); + indexes.should.eql([0, 12, HDPrivateKey.Hardened + 12]); + + indexes = HDPrivateKey._getDerivationIndexes("m/0/12/12'"); + indexes.should.eql([0, 12, HDPrivateKey.Hardened + 12]); + }); + + it('rejects invalid derivation path', function() { + var indexes; + + indexes = HDPrivateKey._getDerivationIndexes("m/"); + expect(indexes).to.be.null; + + indexes = HDPrivateKey._getDerivationIndexes("bad path"); + expect(indexes).to.be.null; + }); }); describe('conversion to plain object/json', function() { From 0beed6efa4be562474bd382b72af891ab581d401 Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Tue, 6 Jan 2015 12:08:42 -0300 Subject: [PATCH 5/6] Refactor HDPublicKey path validation --- lib/hdpublickey.js | 40 +++++++++++++++++----------------------- test/hdpublickey.js | 34 +++++++++++++++++++++++++++------- 2 files changed, 44 insertions(+), 30 deletions(-) diff --git a/lib/hdpublickey.js b/lib/hdpublickey.js index 979faf9..64da85a 100644 --- a/lib/hdpublickey.js +++ b/lib/hdpublickey.js @@ -65,20 +65,23 @@ function HDPublicKey(arg) { } } - /** * Verifies that a given path is valid. * * @param {string|number} arg * @return {boolean} */ -HDPublicKey.prototype.isValidPath = function(arg) { - try { - this.derive(arg); - return true; - } catch (err) { - return false; +HDPublicKey.isValidPath = function(arg) { + if (_.isString(arg)) { + var indexes = HDPrivateKey._getDerivationIndexes(arg); + return indexes !== null && _.all(indexes, HDPublicKey.isValidPath); } + + if (_.isNumber(arg)) { + return arg >= 0 && arg < HDPublicKey.Hardened; + } + + return false; }; /** @@ -149,25 +152,16 @@ HDPublicKey.prototype._deriveFromString = function (path) { /* jshint maxcomplexity: 8 */ if (_.contains(path, "'")) { throw new hdErrors.InvalidIndexCantDeriveHardened(); - } - - var steps = path.split('/'); - - // Special cases: - if (_.contains(HDPublicKey.RootElementAlias, path)) { - return this; - } - if (!_.contains(HDPublicKey.RootElementAlias, steps[0])) { + } else if (!HDPublicKey.isValidPath(path)) { throw new hdErrors.InvalidPath(path); } - steps = steps.slice(1); - var result = this; - for (var step in steps) { - var index = parseInt(steps[step]); - result = result._deriveWithNumber(index); - } - return result; + var indexes = HDPrivateKey._getDerivationIndexes(path); + var derived = indexes.reduce(function(prev, index) { + return prev._deriveWithNumber(index); + }, this); + + return derived; }; /** diff --git a/test/hdpublickey.js b/test/hdpublickey.js index dd73aca..f67b1e4 100644 --- a/test/hdpublickey.js +++ b/test/hdpublickey.js @@ -221,20 +221,40 @@ describe('HDPublicKey interface', function() { }); it('validates correct paths', function() { - var publicKey = new HDPublicKey(xpubkey); - var valid = publicKey.isValidPath('m/123/12'); + var valid; + + valid = HDPublicKey.isValidPath('m/123/12'); valid.should.equal(true); - var valid = publicKey.isValidPath(123, true); + valid = HDPublicKey.isValidPath('m'); + valid.should.equal(true); + + valid = HDPublicKey.isValidPath(123); valid.should.equal(true); }); - it('validates illegal paths', function() { - var publicKey = new HDPublicKey(xpubkey); - var valid = publicKey.isValidPath('m/-1/12'); + it('rejects illegal paths', function() { + var valid; + + valid = HDPublicKey.isValidPath('m/-1/12'); valid.should.equal(false); - var valid = publicKey.isValidPath(HDPublicKey.Hardened); + valid = HDPublicKey.isValidPath("m/0'/12"); + valid.should.equal(false); + + valid = HDPublicKey.isValidPath("m/8000000000/12"); + valid.should.equal(false); + + valid = HDPublicKey.isValidPath('bad path'); + valid.should.equal(false); + + valid = HDPublicKey.isValidPath(-1); + valid.should.equal(false); + + valid = HDPublicKey.isValidPath(8000000000); + valid.should.equal(false); + + valid = HDPublicKey.isValidPath(HDPublicKey.Hardened); valid.should.equal(false); }); From e222ae08c4c6bba92759fce7fcacf1969bb86a44 Mon Sep 17 00:00:00 2001 From: Yemel Jardi Date: Wed, 7 Jan 2015 12:19:41 -0300 Subject: [PATCH 6/6] fix typo --- lib/hdprivatekey.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hdprivatekey.js b/lib/hdprivatekey.js index b8409f8..bad8034 100644 --- a/lib/hdprivatekey.js +++ b/lib/hdprivatekey.js @@ -88,7 +88,7 @@ HDPrivateKey.isValidPath = function(arg, hardened) { /** * Internal function that splits a string path into a derivation index array. * It will return null if the string path is malformed. - * It does not validates if a indexes are in bounds. + * It does not validate if indexes are in bounds. * * @param {string} path * @return {Array}