Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indirectly depends on vulnerable version of bl. #529

Closed
baldo opened this issue Aug 29, 2018 · 9 comments
Closed

Indirectly depends on vulnerable version of bl. #529

baldo opened this issue Aug 29, 2018 · 9 comments

Comments

@baldo
Copy link

baldo commented Aug 29, 2018

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

@cryptix
Copy link
Member

cryptix commented Aug 30, 2018

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
Copy link
Member

cryptix commented Aug 30, 2018

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

@christianbundy
Copy link
Contributor

@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
Copy link
Member

staltz commented Aug 30, 2018

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

@christianbundy
Copy link
Contributor

@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
Copy link
Contributor

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

screenshot from 2018-08-30 11-32-31

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

@cryptix
Copy link
Member

cryptix commented Aug 30, 2018

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

@christianbundy
Copy link
Contributor

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
Copy link

stale bot commented Nov 1, 2018

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.

@stale stale bot added the stale label Nov 1, 2018
@stale stale bot closed this as completed Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants