Security of dependencies

[braydonf]

As of v1.0.2 of bcoin, dependencies are not locked to a specific hash, and can be installed with any matching version regardless of what code there might be associated with that version.

Aside from one's own diligence and review of dependencies, bcoin installation would, at best, inherit the security of the npmjs repository. There have been known with executed attacks against the repository (see https://eslint.org/blog/2018/07/postmortem-for-malicious-package-publishes).

Furthermore, there are many development dependencies that have not been published in over a year, and therefore have not upgraded to use secure hash functions, as there has been known and practices collision attacks against sha1 (see https://shattered.io/) for over a year. So even with a package lockfile there would still be issues.

From discussion with npmjs security about mitigating issues with sha1, there is possible migration process, however it has not yet been made a priority there.

I've opened a PR to add a lockfile and move development dependencies that would be then required to be manually installed, however there hasn't been activity or review (see #569).

What are the next steps?

[braydonf]

Also worth mentioning that some production dependencies have been vendored:

• LevelDB and related at bdb (https://github.com/bcoin-org/bdb)
• Faye Websocket at bsock (https://github.com/bcoin-org/bsock)

However these versions are not yet locked down in the bcoin package-lock.json or npm-shrinkwrap.json. Thus it's currently possible to keep an uncommitted package-lock.json file for production dependencies without sha1 issues.

[braydonf]

So I've looked at forking mocha (as mentioned at #628 (comment)) with vendored dependencies at https://github.com/braydonf/mocha/commits/vendor and the associated bcoin branch using that version is at https://github.com/braydonf/bcoin/commits/lockfile

There are 23 dependencies for mocha, here is a breakdown of the lines:

diff			2762 lines
commander 		1536 lines
glob 			1516 lines
minimatch		923 lines
brance-expansion 	200 lines
debug 			615 lines
fs.realpath 		369 lines
he 			342 lines
growl 			340 lines
minimist		187 lines
ms			162 lines
supports-color		136 lines
mkdirp			98 lines
balanced-match 		59 lines
inflight 		54 lines
once			42 lines
wrappy			33 lines
inherits		30 lines
browser-stdout 		25 lines
path-is-absolute	20 lines
concat-map 		13 lines
escape-string-regexp 	11 lines
has-flag 		9 lines

That's not including the development dependencies for mocha which go over 2000, and several with known vulnerabilities, which can't be fixed by upgrading to the latest versions of each of the development dependencies.

[braydonf]

Here is another view of mocha dependencies based on maintenance (last updated):

commander 		(2 months ago)
debug 			(2 months ago)
he 			(2 months ago)
glob 			(3 months ago)
supports-color 		(3 months ago)
browser-stdout 		(9 months ago)
diff 			(9 months ago)
growl 			(8 months ago)
brace-expansion 	(10 months ago)
balanced-match 		(1 year ago)
has-flag 		(1 year ago)
ms 			(1 year ago)
inflight 		(2 years ago)
once 			(2 years ago)
minimatch 		(2 years ago)
fs.realpath 		(2 years ago)
inherits 		(2 years ago)
minimist 		(3 years ago)
wrappy 			(3 years ago)
escape-string-regexp 	(3 years ago)
mkdirp 			(4 years ago)
concat-map 		(5 years ago)
path-is-absolute 	(deprecated)

• Only 22% of them have been updated in the last 6 months.
• 39% updated in the last year.
• 61% in at least the last two years.

[chjj]

Mmm, mocha is messier than I thought it was. I started working on a minimal reimplementation: https://gist.github.com/chjj/4fc87c2b3e882c9d240a544488639f7e

It's quick and dirty, but it's good enough to run the entire bcoin test suite. Don't know if we want to pursue this more.

[braydonf]

Okay cool, was taking a look at something similar based on a minimal test bootstrap from: https://github.com/braydonf/chainbeacon/blob/master/test/bootstrap.js

[chjj]

Package up here: https://github.com/chjj/bmocha

I've been testing it all day and it seems to work. I think I'm going to start migrating packages over to it. I figure if it turns out it's somehow broken, we can just point the bmocha repo to your vendored mocha.

[braydonf]

I tried in on one of my work-in-progress branches (#605) and ran into a few issues. describe blocks inside describe blocks, for group subsets of tests on one. Missing .skip was another. Also it may be a good idea to run the tests as a child process so that there isn't global state leaks between tests. There may be other issues, but those stood out the most.

[pinheadmz]

Just pulled master df1e8c0

reinstalled: rm -rf node_modules; npm i -g

npm run test:

  1) Wallet
       should fill tx with inputs when encrypted:

      AssertionError [ERR_ASSERTION]: undefined == true
      + expected - actual

      -undefined
      +true

      at Object.it (/Users/pinhead/Desktop/work/bcoin/test/wallet-test.js:1044:5)
      at <anonymous>
      at process._tickCallback (internal/process/next_tick.js:160:7)

...but this only happened once and it will not reproduce! Reinstalling and running individual tests is all fine. ✅ No error recurrence since the first run test.

I have had tests RANDOMLY fail before -- Javed fixed the last case: #529