ssbc / ssb-server

The gossip and replication server for Secure Scuttlebutt - a distributed social network
1.69k stars 164 forks source link

npm audit reveals vulnerabilities #686

Closed garbados closed 4 years ago

garbados commented 5 years ago

Installing ssb-server in a project, npm makes a lot of noise:

$ npm i ssb-server
...
+ ssb-server@15.1.0
added 1097 packages from 772 contributors and audited 9992 packages in 10.028s
found 81 vulnerabilities (5 low, 1 moderate, 75 high)
  run `npm audit fix` to fix them, or `npm audit` for details

Running npm audit fix only fixes some of these vulnerabilities. I invite you to take a look at the npm audit output yourself -- it's significant, and doesn't gist well.

christianbundy commented 5 years ago

Hey @garbados! :wave:

Thanks for opening this issue. Running npm audit fix seems to resolve most of the potential problems for me, but I'm still seeing these:

click to expand ``` ───────────────┬──────────────────────────────────────────────────────────────┐ │ Moderate │ Memory Exposure │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ bl │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=0.9.5 <1.0.0 || >=1.0.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ssb-invite │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ssb-invite > level-sublevel > levelup > bl │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/596 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Low │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=2.3.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ mdmanifest │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ mdmanifest > remark > chokidar > anymatch > micromatch > │ │ │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/786 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Low │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=2.3.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ssb-db │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ssb-db > mdmanifest > remark > chokidar > anymatch > │ │ │ micromatch > braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/786 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Low │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=2.3.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ssb-gossip │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ssb-gossip > mdmanifest > remark > chokidar > anymatch > │ │ │ micromatch > braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/786 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Low │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=2.3.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ssb-invite │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ssb-invite > mdmanifest > remark > chokidar > anymatch > │ │ │ micromatch > braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/786 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ┌───────────────┬──────────────────────────────────────────────────────────────┐ │ Low │ Regular Expression Denial of Service │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Package │ braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Patched in │ >=2.3.1 │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Dependency of │ ssb-plugins │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ Path │ ssb-plugins > mdmanifest > remark > chokidar > anymatch > │ │ │ micromatch > braces │ ├───────────────┼──────────────────────────────────────────────────────────────┤ │ More info │ https://npmjs.com/advisories/786 │ └───────────────┴──────────────────────────────────────────────────────────────┘ ```

I don't think the regex DoS problems will have any effect on us, but I don't know enough about the memory exposure problem to conclusively say it's fine. It looks like we're inheriting that from @dominictarr's level-sublevel, which is unmaintained, so we may need to fix ssb-invite so that it's not running on unmaintained code anymore.

austinfrey commented 5 years ago

levelup might only be a devDependency in level-sublevel? it would still be good to keep it up to date though. subleveldown is another option, but i don't think it has some key features that level-sublevel is using, like hooks.

does unmaintained mean, the module is finished so no need to change, or does it mean that this module should be phased out when possible?

christianbundy commented 5 years ago

does unmaintained mean, the module is finished so no need to change, or does it mean that this module should be phased out when possible?

I think the module should be removed when possible, replaced with subleveldown if we actually need sub-databases in level. See this issue for some more info, it looks like were conflicting bugfixes that caused other bugs so the changes were rolled back and the module was deprecated.

dominictarr commented 5 years ago

These are all false positives. to be vulnerable to a regexp DoS, you'd have to run a user provided regular expression, which we don't. Also to get the memory exposure, you'd have to have a user request for some size of memory and return it, which we also don't do.

stale[bot] commented 4 years ago

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?