sintaxi / surge

CLI for the surge.sh CDN
https://surge.sh
2.85k stars 136 forks source link

indirect dependancy on minimatch causes a vulnerability #342

Closed driesdesmet closed 5 years ago

driesdesmet commented 5 years ago

This is due to fstream-ignore, which seems not te get updated lately.

$ npm install
added 52 packages from 46 contributors and audited 11984 packages in 35.632s
found 2 vulnerabilities (1 low, 1 high)
  run `npm audit fix` to fix them, or `npm audit` for details

$ npm audit

                       === npm audit security report ===                        

┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Regular Expression Denial of Service                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimatch                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=3.0.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ surge [dev]                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ surge > fstream-ignore > minimatch                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/118                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ lodash                                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.17.5                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ surge [dev]                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ surge > cli-table2 > lodash                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/577                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 2 vulnerabilities (1 low, 1 high) in 11984 scanned packages
  2 vulnerabilities require manual review. See the full report for details.
driesdesmet commented 5 years ago

Looking more into this, I can see that surge has pinned the version of fstream-ignore to 1.0.2, which depends on minimatch 2.x.x. The latest version of fstream-ignore seems to be 1.0.5, depending on minimatch 3.x.x which would resolve this issue. Is there a known incompatiblity?

xmclark commented 5 years ago

Github gave me a very obnoxious notification about lodash 3.10.1 when I added surge as a dev dependency to one of my projects.

I tried to trace down the dependency but I could not find it.

irrg commented 5 years ago

@sintaxi I'm also checking in on this and I'm kind of surprised that this isn't being addressed? The minimatch vulnerability is rated 'high' by npm audit. That…should be a thing.

sintaxi commented 5 years ago

Much appreciated gents. Thanks for staying on top of this and bringing it to my attention again. As @driesdesmet points out there was a pin a one point because our integration tests were failing with updated libs.

I'll bump this up the priority list to see if I can get to the bottom of it.

irrg commented 5 years ago

I was able to bump the version on fstream-ignore to 1.0.5, and set the dependency for cli-table2 to the github url (jamestalmage/cli-table2) to get rid of the vulnerability alerts.

cli-table2, while the cause of a minor warning might be more iffy—it hasn't been published in a while and the code in the repo is different from what's published (the old lodash version that causes the vulnerability wasn't being used, so it was just removed in the repo) while still having a 0.2.0 version in the package.json.

I was able to run mocha tests locally without issues the first time but I'm seeing timeouts and timeout related async warnings when I run it again. test:local never succeeds but it's for the same reasons—timeouts and asyncs.

jsomsanith-tlnd commented 5 years ago

Hello guys. Any update on that ? The version of minimatch has been upgraded in fstream-ignore since version 1.0.3. Is there any plan to upgrade ?

sintaxi commented 5 years ago

Fixed. Thanks to everyone for waiting on this and nudging me from time to time. v0.20.3 has been released.

jsomsanith-tlnd commented 5 years ago

Thank you :)