Should NPM audit security vulnerabilities taken seriously?

cileria commented 2 years ago

Describe the bug I made a package.json setup that installs the latest packages of storybook for my webapp and I get an alarming number of security issues.

To Reproduce I set up a node project that installs the following packages:

"@storybook/addon-actions": "^6.4.9", "@storybook/addon-essentials": "^6.4.9", "@storybook/addon-links": "^6.4.9", "@storybook/node-logger": "^6.4.9", "@storybook/preset-create-react-app": "^3.2.0", "@storybook/react": "^6.4.9"

and got

43 vulnerabilities (23 moderate, 18 high, 2 critical)

json-derulo commented 2 years ago

There is an interesting article about this topic: npm audit: Broken by Design

VasylBoyko commented 2 years ago

IMHO, 18 high and 2 critical MUST be fixed.

joaogarin commented 2 years ago

I do think they should be taken seriously. Right now I am seeing a huge amount of npm vulnerabilities coming from storybook's packages...

from this PR https://github.com/forumone/gesso/pull/564 looks like it will be included in 6.5 is that it?

humarkx commented 2 years ago

Any news on this? The list is growing, a lot of high severity vulnerabilities


│ high          │ Regular expression denial of service in glob-parent          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-essentials                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-essentials > @storybook/core-common >       │
│               │ webpack > watchpack > watchpack-chokidar2 > chokidar >       │
│               │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1067329                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular expression denial of service in glob-parent          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-essentials                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-essentials > @storybook/addon-controls >    │
│               │ @storybook/core-common > webpack > watchpack >               │
│               │ watchpack-chokidar2 > chokidar > glob-parent                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1067329                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular expression denial of service in glob-parent          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/react                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/react > @storybook/core > @storybook/core-server  │
│               │ > @storybook/core-common > webpack > watchpack >             │
│               │ watchpack-chokidar2 > chokidar > glob-parent                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1067329                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular expression denial of service in glob-parent          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=5.1.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/react                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/react > @storybook/core > @storybook/core-server  │
│               │ > @storybook/builder-webpack4 > @storybook/core-common >     │
│               │ webpack > watchpack > watchpack-chokidar2 > chokidar >       │
│               │ glob-parent                                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1067329                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular Expression Denial of Service in trim-newlines        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim-newlines                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=3.0.1                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/react                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/react > @storybook/core > @storybook/core-server  │
│               │ > x-default-browser > default-browser-id > meow >            │
│               │ trim-newlines                                                │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1067858                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular Expression Denial of Service in trim                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.0.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-essentials                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-essentials > @storybook/addon-docs >        │
│               │ @storybook/mdx1-csf > @mdx-js/mdx > remark-parse > trim      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1068044                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular Expression Denial of Service in trim                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.0.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/addon-essentials                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/addon-essentials > @storybook/addon-docs >        │
│               │ @storybook/mdx1-csf > @mdx-js/mdx > remark-mdx >             │
│               │ remark-parse > trim                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1068044                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular Expression Denial of Service in trim                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.0.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/react                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/react > @storybook/core > @storybook/core-server  │
│               │ > @storybook/csf-tools > @storybook/mdx1-csf > @mdx-js/mdx > │
│               │ remark-parse > trim                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1068044                     │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Regular Expression Denial of Service in trim                 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ trim                                                         │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.0.3                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ @storybook/react                                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ @storybook/react > @storybook/core > @storybook/core-server  │
│               │ > @storybook/csf-tools > @storybook/mdx1-csf > @mdx-js/mdx > │
│               │ remark-mdx > remark-parse > trim                             │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://www.npmjs.com/advisories/1068044       ```

shilman commented 2 years ago

@humarkx we're upgrading everything as part of 7.0 and will clear the decks with that. furthermore, we'll be doing more frequent major releases after 7.0 to keep the list down. thanks for your patience!

janaagaard75 commented 2 years ago

Sounds really great, @shilman. One note, though: If you want to get to the bottom of this, it won't be enough to just upgrade the packages. You will also have to shred some of the old packages that Storybook relies on.

Example: The just released Storybook 6.5 adds x-default-browser as a dependency. That package hasn't been updated in five years! So it does - of course - come with a security issue.

shilman commented 2 years ago

@janaagaard75 yes, we will be dealing with it e.g. https://github.com/storybookjs/storybook/pull/18277

jinder commented 1 year ago

@shilman how long before 7 ships? Storybook packages are getting flagged by our internal audit process, and if this is still going to take months we may need to bin storybook entirely.

janaagaard75 commented 1 year ago

Storybook 7 improves on the number of vulnerabilities, but they do remove them all.

Storybook version	Vulnerabilities reported by `npm audit`	Date of audit
6.5.9	18 vulnerabilities (9 moderate, 9 high)	2022-07-22
6.5.10	18 vulnerabilities (5 moderate, 13 high)	2022-10-04
7.0.0-alpha.13	11 vulnerabilities (5 moderate, 6 high)	2022-07-22
7.0.0-alpha.29	11 vulnerabilities (5 moderate, 6 high)	2022-10-04

We were able to keep using Storybook by arguing that our Storybook pages are deployed separately from the rest of the app, that they aren't critical, and that a lot of Storybook's logic is done in the compilation step, so a vulnerable dependency isn't necessarily included in the compiled code, and mentioning some of points from npm audit: Broken by Design.

SkyCodeSky commented 1 year ago

Are there any updates yet? It would be great if there would be 0 vulnerabilities with version 7.0. :-)

KyleTryon commented 1 year ago

I am currently showing 21 High vulnerabilities.

shilman commented 1 year ago

@janaagaard75 storybook 7 is still WIP. i hope to get rid of the high severity vulnerabilities before we ship.

Etzix commented 1 year ago

Remember that you only have to have Storybook as a dev dependency, so aslong as you skip dev when doing audits, your repos should be fine.

shilman commented 1 year ago

AFAIK all high severity npm audit issues are fixed in SB7

To upgrade to the latest prerelease:

npx sb@next upgrade --prerelease

datumgeek commented 1 year ago

your upgrade process is amazing

v11ncent commented 1 year ago

I am currently showing 21 High vulnerabilities.

I am as well. 6 from create-react-app and 21 from npx storybook init. Steps taken: npx create-react-app <folder>, cd <folder>, npx storybook init.

iamtheprogram commented 1 year ago

Same happened to me today after npx storybook init in my project. The project was created with npm init.

v11ncent commented 1 year ago

Same happened to me today after npx storybook init in my project. The project was created with npm init.

I discussed with a maintainer on the official Storybook discord server about the vulnerabilities. If you upgrade to Storybook 7.0 beta, it reduces the amount of errors from 21 high severity errors, down to 3 moderate & 3 high severity errors. There is currently a PR in the works about updating some modules to remove these security vulnerabilities. See: https://github.com/storybookjs/storybook/issues/18155#issuecomment-1433054958.

storybookjs / storybook

Should NPM audit security vulnerabilities taken seriously? #17220