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

Should NPM audit security vulnerabilities taken seriously? #17220

Closed
cileria opened this issue Jan 12, 2022 · 18 comments
Closed

Should NPM audit security vulnerabilities taken seriously? #17220

cileria opened this issue Jan 12, 2022 · 18 comments

Comments

@cileria
Copy link

cileria commented Jan 12, 2022

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

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

@VasylBoyko
Copy link

IMHO, 18 high and 2 critical MUST be fixed.

@joaogarin
Copy link

joaogarin commented Feb 17, 2022

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 forumone/gesso#564 looks like it will be included in 6.5 is that it?

@humarkx
Copy link

humarkx commented Apr 26, 2022

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

shilman commented Apr 28, 2022

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

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

shilman commented May 20, 2022

@janaagaard75 yes, we will be dealing with it e.g. #18277

@jinder
Copy link

jinder commented Jul 21, 2022

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

janaagaard75 commented Jul 22, 2022

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

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

@KyleTryon
Copy link

I am currently showing 21 High vulnerabilities.

@shilman
Copy link
Member

shilman commented Sep 10, 2022

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

@Etzix
Copy link

Etzix commented Oct 31, 2022

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.

Copy link
Member

shilman commented Oct 31, 2022

AFAIK all high severity npm audit issues are fixed in SB7

To upgrade to the latest prerelease:

npx sb@next upgrade --prerelease

@shilman shilman closed this as completed Oct 31, 2022
@datumgeek
Copy link

your upgrade process is amazing

@v11ncent
Copy link

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

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

@v11ncent
Copy link

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: #18155 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests