Refactor HDPrivateKey path validation

lib/hdprivatekey.js

@@ -69,15 +69,51 @@ function HDPrivateKey(arg) {
  * @param {boolean?} hardened
  * @return {boolean}
  */
-HDPrivateKey.prototype.isValidPath = function(arg, hardened) {
+HDPrivateKey.isValidPath = function(arg, hardened) {
-  try {
+  if (_.isString(arg)) {
-    this.derive(arg, hardened);
+    var indexes = HDPrivateKey._getDerivationIndexes(arg);
-    return true;
+    return indexes !== null && _.all(indexes, HDPrivateKey.isValidPath);
-  } catch (err) {
-    return false;
   }
+
+  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) {
+  if (!HDPrivateKey.isValidPath(index, hardened)) {
-    hardened = true;
-  }
-
-  if (index < HDPrivateKey.Hardened && hardened) {
-    index += HDPrivateKey.Hardened;
-  }
-
-  if (index < 0 || index >= HDPrivateKey.MaxIndex) {
     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('/');
+  if (!HDPrivateKey.isValidPath(path)) {
-
-  // Special cases:
-  if (_.contains(HDPrivateKey.RootElementAlias, path)) {
-    return this;
-  }
-  if (!_.contains(HDPrivateKey.RootElementAlias, steps[0])) {
     throw new hdErrors.InvalidPath(path);
   }
-  steps = steps.slice(1);
 
-  var result = this;
+  var indexes = HDPrivateKey._getDerivationIndexes(path);
-  for (var step in steps) {
+  var derived = indexes.reduce(function(prev, index) {
-    var index = parseInt(steps[step]);
+    return prev._deriveWithNumber(index);
-    var hardened = steps[step] !== index.toString();
+  }, this);
-    result = result._deriveWithNumber(index, hardened);
+
-  }
+  return derived;
-  return result;
 };
 
 /**

test/hdprivatekey.js

@@ -190,22 +190,70 @@ describe('HDPrivate key interface', function() {
     derivedByNumber.xprivkey.should.equal(derivedByString.xprivkey);
   });
 
-  it('validates correct paths', function() {
+  describe('validates paths', function() {
-    var privateKey = new HDPrivateKey(xprivkey);
+    it('validates correct paths', function() {
-    var valid = privateKey.isValidPath('m/0\'/1/2\'');
+      var valid;
-    valid.should.equal(true);
 
-    var valid = privateKey.isValidPath(123, true);
+      valid = HDPrivateKey.isValidPath("m/0'/1/2'");
-    valid.should.equal(true);
+      valid.should.equal(true);
-  });
 
-  it('validates illegal paths', function() {
+      valid = HDPrivateKey.isValidPath('m');
-    var privateKey = new HDPrivateKey(xprivkey);
+      valid.should.equal(true);
-    var valid = privateKey.isValidPath('m/-1/12');
-    valid.should.equal(false);
 
-    var valid = privateKey.isValidPath(HDPrivateKey.MaxHardened);
+      valid = HDPrivateKey.isValidPath(123, true);
-    valid.should.equal(false);
+      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() {