fix: catch error when unmarshaling instead of crashing (#72)

Author: mkg20001

.gitignore

@@ -1,3 +1,4 @@
+docs
 **/node_modules/
 **/*.log
 test/repo-tests*
@@ -41,4 +42,4 @@ test/test-data/go-ipfs-repo/LOG.old
 
 # while testing npm5
 package-lock.json
-yarn.lock
+yarn.lock

package.json

@@ -33,16 +33,16 @@
   },
   "homepage": "https://github.com/libp2p/js-peer-id",
   "devDependencies": {
-    "aegir": "^12.0.8",
+    "aegir": "^12.2.0",
     "chai": "^4.1.2",
     "dirty-chai": "^2.0.1",
     "pre-commit": "^1.2.2"
   },
   "dependencies": {
-    "async": "^2.5.0",
-    "libp2p-crypto": "~0.10.3",
+    "async": "^2.6.0",
+    "libp2p-crypto": "~0.10.4",
     "lodash": "^4.17.4",
-    "multihashes": "~0.4.9"
+    "multihashes": "~0.4.12"
   },
   "repository": {
     "type": "git",

src/index.js

@@ -165,12 +165,20 @@ exports.createFromPubKey = function (key, callback) {
     throw new Error('callback is required')
   }
 
-  let buf = key
-  if (typeof buf === 'string') {
-    buf = Buffer.from(key, 'base64')
-  }
+  let pubKey
+
+  try {
+    let buf = key
+    if (typeof buf === 'string') {
+      buf = Buffer.from(key, 'base64')
+    }
 
-  const pubKey = crypto.keys.unmarshalPublicKey(buf)
+    if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer')
+
+    pubKey = crypto.keys.unmarshalPublicKey(buf)
+  } catch (err) {
+    return callback(err)
+  }
 
   pubKey.hash((err, digest) => {
     if (err) {
@@ -183,15 +191,22 @@ exports.createFromPubKey = function (key, callback) {
 
 // Private key input will be a string
 exports.createFromPrivKey = function (key, callback) {
-  let buf = key
-  if (typeof buf === 'string') {
-    buf = Buffer.from(key, 'base64')
-  }
-
   if (typeof callback !== 'function') {
     throw new Error('callback is required')
   }
 
+  let buf = key
+
+  try {
+    if (typeof buf === 'string') {
+      buf = Buffer.from(key, 'base64')
+    }
+
+    if (!Buffer.isBuffer(buf)) throw new Error('Supplied key is neither a base64 string nor a buffer')
+  } catch (err) {
+    return callback(err)
+  }
+
   waterfall([
     (cb) => crypto.keys.unmarshalPrivateKey(buf, cb),
     (privKey, cb) => privKey.public.hash((err, digest) => {
@@ -211,10 +226,19 @@ exports.createFromJSON = function (obj, callback) {
     throw new Error('callback is required')
   }
 
-  const id = mh.fromB58String(obj.id)
-  const rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64')
-  const rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64')
-  const pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey)
+  let id
+  let rawPrivKey
+  let rawPubKey
+  let pub
+
+  try {
+    id = mh.fromB58String(obj.id)
+    rawPrivKey = obj.privKey && Buffer.from(obj.privKey, 'base64')
+    rawPubKey = obj.pubKey && Buffer.from(obj.pubKey, 'base64')
+    pub = rawPubKey && crypto.keys.unmarshalPublicKey(rawPubKey)
+  } catch (err) {
+    return callback(err)
+  }
 
   if (rawPrivKey) {
     waterfall([

test/peer-id.spec.js

@@ -12,6 +12,8 @@ const parallel = require('async/parallel')
 
 const PeerId = require('../src')
 
+const util = require('util')
+
 const testId = require('./fixtures/sample-id')
 const testIdHex = testId.id
 const testIdBytes = mh.fromHexString(testId.id)
@@ -245,6 +247,24 @@ describe('PeerId', () => {
     })
   })
 
+  describe('returns error via cb instead of crashing', () => {
+    const garbage = [Buffer.from('00010203040506070809', 'hex'), {}, null, false, undefined, true, 1, 0, Buffer.from(''), 'aGVsbG93b3JsZA==', 'helloworld', '']
+
+    const fncs = ['createFromPubKey', 'createFromPrivKey', 'createFromJSON']
+
+    garbage.forEach(garbage => {
+      fncs.forEach(fnc => {
+        it(fnc + '(' + util.inspect(garbage) + ')', cb => {
+          PeerId[fnc](garbage, (err, res) => {
+            expect(err).to.exist()
+            expect(res).to.not.exist()
+            cb()
+          })
+        })
+      })
+    })
+  })
+
   describe('throws on inconsistent data', () => {
     let k1
     let k2