Indirectly depends on vulnerable version of bl.

[baldo]

Via: scuttlebot > secure-scuttlebutt > level-sublevel > levelup > bl. For details see: https://nodesecurity.io/advisories/596

[cryptix]

I'm having a hard time figuring out which version of sublevel isn't affected by this anymore. I can't see it being required directly but doing npm ls bl in scuttlebot gives me two paths to it, both through sublevel (so maybe this issue should be over there but we are here already...).

@arj03 I think you were down that stack quite far with the performance improvments you did. Can you shed some light on this?

[cryptix]

cc @christianbundy I think you might came across this as well with the leveldb vs node10 stuff?

[christianbundy]

@cryptix I'm confused by this -- the latest level-sublevel from the GitHub repo contains version 6.6.4, but there's a version 6.6.5 on NPM with a dependency change that introduces the old version of levelup:

3,5c3
<   "level-codec": "^9.0.0",
<   "level-errors": "^2.0.0",
<   "level-iterator-stream": "^2.0.3",
---
>   "levelup": "~0.19.0",

@dominictarr Any chance you know what the difference is between 6.6.4 in the repo and 6.6.5 on NPM? I'd love to help get this resolved if possible.

[staltz]

Is it possible to do diff by downloading 6.6.4 from the repo and 6.6.5 from npm and comparing?

[christianbundy]

@staltz Red is 6.6.4 from the repo, green is 6.6.5 from NPM:

diff --git a/README.md b/README.md
index 3da1b29..1d78377 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@ Separate sections of levelup, with hooks!
 
 [![testling badge](https://ci.testling.com/dominictarr/level-sublevel.png)](https://ci.testling.com/dominictarr/level-sublevel)
 
-This module allows you to create separate sections of a
+This module allows you to create seperate sections of a
 [levelup](https://github.com/rvagg/node-levelup) database,
 kinda like tables in an sql database, but evented, and ranged,
 for real-time changing data.
@@ -14,7 +14,7 @@ for real-time changing data.
 ## level-sublevel@6 **BREAKING CHANGES**
 
 The long awaited `level-sublevel` rewrite is out!
-You are hereby warned this is a _significant breaking_ change.
+You are hearby warned this is a _significant breaking_ change.
 So it's good to use it with a new project,
 The user api is _mostly_ the same as before,
 but the way that keys are _encoded_ has changed, and _this means
diff --git a/bytewise.js b/bytewise.js
index 1396745..cc6f985 100644
--- a/bytewise.js
+++ b/bytewise.js
@@ -1,9 +1,9 @@
 var nut     = require('./nut')
 var shell   = require('./shell') //the shell surrounds the nut
-var Codec   = require('level-codec')
+var codec   = require('levelup/lib/codec')
 var merge   = require('xtend')
 var compare = require('typewiselite')
-var ReadStream = require('./read-stream')
+var ReadStream = require('levelup/lib/read-stream')
 
 var precodec = require('./codec/bytewise')
 
@@ -22,7 +22,7 @@ module.exports = function (db, opts) {
   }, opts)
 
   return shell (
-    nut ( db, precodec, new Codec, compare ),
+    nut ( db, precodec, codec, compare ),
     [], ReadStream, opts
   )
 }
diff --git a/index.js b/index.js
index e680623..4b0c8d4 100644
--- a/index.js
+++ b/index.js
@@ -4,14 +4,14 @@
 var nut   = require('./nut')
 var shell = require('./shell') //the shell surrounds the nut
 var precodec = require('./codec')
-var Codec = require('level-codec')
+var codec = require('levelup/lib/codec')
 var merge = require('xtend')
 
-var ReadStream = require('./read-stream')
+var ReadStream = require('levelup/lib/read-stream')
 
 var sublevel = function (db, opts) {
   opts = merge(db.options, opts)
-  return shell ( nut ( db, precodec, new Codec ), [], ReadStream, opts)
+  return shell ( nut ( db, precodec, codec ), [], ReadStream, opts)
 }
 
 module.exports = function (db, opts) {
diff --git a/legacy.js b/legacy.js
index 0112970..06debfd 100644
--- a/legacy.js
+++ b/legacy.js
@@ -1,9 +1,9 @@
 var nut   = require('./nut')
 var shell = require('./shell') //the shell surrounds the nut
-var Codec = require('level-codec')
+var codec = require('levelup/lib/codec')
 var merge = require('xtend')
 
-var ReadStream = require('./read-stream')
+var ReadStream = require('levelup/lib/read-stream')
 
 var precodec = require('./codec/legacy')
 
@@ -11,7 +11,7 @@ module.exports = function (db, opts) {
 
   opts = merge(db.options, opts)
 
-  return shell ( nut ( db, precodec, new Codec ), [], ReadStream, db.options)
+  return shell ( nut ( db, precodec, codec ), [], ReadStream, db.options)
 
 }
 
diff --git a/nut.js b/nut.js
index c4aa118..e10613e 100644
--- a/nut.js
+++ b/nut.js
@@ -113,7 +113,7 @@ module.exports = function (db, precodec, codec, compare) {
         cb()
     },
     get: function (key, prefix, opts, cb) {
-      opts.asBuffer = codec.valueAsBuffer(opts)
+      opts.asBuffer = codec.isValueAsBuffer(opts)
       return (db.db || db).get(
         encodePrefix(prefix, key, opts),
         opts,
@@ -185,7 +185,7 @@ module.exports = function (db, precodec, codec, compare) {
         opts.limit = -1
 
       opts.keyAsBuffer = precodec.buffer
-      opts.valueAsBuffer = codec.valueAsBuffer(opts)
+      opts.valueAsBuffer = codec.isValueAsBuffer(opts)
 
       function wrapIterator (iterator) {
         return {
diff --git a/package.json b/package.json
index a2fd284..38111a2 100644
--- a/package.json
+++ b/package.json
@@ -1,17 +1,44 @@
 {
-  "name": "level-sublevel",
-  "description": "partition levelup databases",
-  "version": "6.6.4",
-  "homepage": "https://github.com/dominictarr/level-sublevel",
-  "repository": {
-    "type": "git",
-    "url": "git://github.com/dominictarr/level-sublevel.git"
+  "_from": "level-sublevel@6.6.5",
+  "_id": "level-sublevel@6.6.5",
+  "_inBundle": false,
+  "_integrity": "sha512-SBSR60x+dghhwGUxPKS+BvV1xNqnwsEUBKmnFepPaHJ6VkBXyPK9SImGc3K2BkwBfpxlt7GKkBNlCnrdufsejA==",
+  "_location": "/level-sublevel",
+  "_phantomChildren": {
+    "core-util-is": "1.0.2",
+    "errno": "0.1.7",
+    "inherits": "2.0.3"
+  },
+  "_requested": {
+    "type": "version",
+    "registry": true,
+    "raw": "level-sublevel@6.6.5",
+    "name": "level-sublevel",
+    "escapedName": "level-sublevel",
+    "rawSpec": "6.6.5",
+    "saveSpec": null,
+    "fetchSpec": "6.6.5"
+  },
+  "_requiredBy": [
+    "#DEV:/",
+    "#USER"
+  ],
+  "_resolved": "https://registry.npmjs.org/level-sublevel/-/level-sublevel-6.6.5.tgz",
+  "_shasum": "acddfa2be033b9e503544e2c647f3c03d5a23fbd",
+  "_spec": "level-sublevel@6.6.5",
+  "_where": "/home/christianbundy/Source/scuttlebot",
+  "author": {
+    "name": "Dominic Tarr",
+    "email": "dominic.tarr@gmail.com",
+    "url": "http://dominictarr.com"
   },
+  "bugs": {
+    "url": "https://github.com/dominictarr/level-sublevel/issues"
+  },
+  "bundleDependencies": false,
   "dependencies": {
     "bytewise": "~1.1.0",
-    "level-codec": "^9.0.0",
-    "level-errors": "^2.0.0",
-    "level-iterator-stream": "^2.0.3",
+    "levelup": "~0.19.0",
     "ltgt": "~2.1.1",
     "pull-defer": "^0.2.2",
     "pull-level": "^2.0.3",
@@ -19,21 +46,29 @@
     "typewiselite": "~1.0.0",
     "xtend": "~4.0.0"
   },
+  "deprecated": false,
+  "description": "partition levelup databases",
   "devDependencies": {
     "level": "^3.0.1",
     "level-test": "^3.0.0",
     "monotonic-timestamp": "0.0.8",
+    "pull-level": "~1.1.1",
     "rimraf": "~2.1.4",
     "shasum": "0.0.2",
     "stream-to-pull-stream": "~1.2.0",
-    "tape": "^4.9.1",
+    "tape": "~2.14.0",
     "through": "~2.3.4"
   },
+  "homepage": "https://github.com/dominictarr/level-sublevel",
+  "license": "MIT",
+  "name": "level-sublevel",
+  "repository": {
+    "type": "git",
+    "url": "git://github.com/dominictarr/level-sublevel.git"
+  },
   "scripts": {
-    "test": "node test-runner.js"
+    "test": "set -e; for t in test/*.js; do node $t; done"
   },
-  "author": "Dominic Tarr <dominic.tarr@gmail.com> (http://dominictarr.com)",
-  "license": "MIT",
   "stability": "unstable",
   "testling": {
     "files": "test/*.js",
@@ -50,5 +85,6 @@
       "iphone/6.0..latest",
       "android-browser/4.2..latest"
     ]
-  }
+  },
+  "version": "6.6.5"
 }
diff --git a/shell.js b/shell.js
index a39a584..4275f21 100644
--- a/shell.js
+++ b/shell.js
@@ -1,7 +1,7 @@
 var EventEmitter = require('events').EventEmitter
 var addpre = require('./range').addPrefix
 
-var errors = require('level-errors')
+var errors = require('levelup/lib/errors')
 
 function isFunction (f) {
   return 'function' === typeof f
diff --git a/test/level.js b/test/level.js
index 7ca9f32..61c4cd2 100644
--- a/test/level.js
+++ b/test/level.js
@@ -6,7 +6,7 @@ var path = require('path')
 var mock  = require('./mock')
 var nut   = require('../nut')
 var shell = require('../shell') //the shell surrounds the nut
-var Codec = require('level-codec')
+var codec = require('levelup/lib/codec')
 var concat = require('../codec')
 var legacy = require('../codec/legacy')
 var bytewise = require('../codec/bytewise')
@@ -23,7 +23,7 @@ var pullReadStream = require('../pull')
 function create (precodec, db) {
 
   //convert pull stream to iterators
-  return shell ( nut ( db || mock(), precodec, new Codec ), [], pullReadStream)
+  return shell ( nut ( db || mock(), precodec, codec ), [], pullReadStream)
 }
 
 function prehookPut (db) {

My method, in case it's useful (or I messed something up):

$ ls
level-sublevel	scuttlebot
$ cp -r scuttlebot/node_modules/level-sublevel/* level-sublevel
$ cd level-sublevel
$ git diff

IIRC cp -r * doesn't copy dotfiles, but I'm not sure that that matters in this case.

[christianbundy]

Just looked at the branches and it looks like 6.6.2 was re-released as 6.6.5 with no changes.

If this was a mistake, I think our best option is to re-release 6.6.4 as 6.6.6.

[cryptix]

This is really irritating but I don't think we will get an definitive answer before next week.

[christianbundy]

I totally forgot that Dominic was gone -- I've now contributed to his nightmare inbox. My bad.

It looks like bl is being used here, and I think we have two options:

• Change package.json in secure-scuttlebutt to use "level-sublevel": "6.6.4" without a caret/tilde.
• Wait for Dominic to get back and republish as 6.6.6.

I've read a handful of threads to try to determine whether this vulnerability actually has any negative effect, and this comment was the most informative. My understanding is that we're only vulnerable if someone can pass a JavaScript Number through scuttlebot -> secure-scuttlebutt -> level-sublevel -> levelup -> WriteStream.write(), which I'm guessing isn't doable but I'm not familiar enough to say it's impossible.

My personal preference would be to err on the side of caution and change the secure-scuttlebutt dependency until we can hear back from Dominic, but I'd be comfortable just waiting it out if there are any downsides to that approach.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.