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

Vulnerability on @storybook/addon-docs related with trim #14603

Closed
BiancaArtola opened this issue Apr 14, 2021 · 42 comments
Closed

Vulnerability on @storybook/addon-docs related with trim #14603

BiancaArtola opened this issue Apr 14, 2021 · 42 comments

Comments

@BiancaArtola
Copy link

I know that it is not a bug, but I need to report a vulnerability that @storybook/addon-docs has.

image

The following is the related dependencie tree:
@storybook/addon-docs -> @mdx-js/loader -> @mdx-js/mdx -> remark-mdx -> remark-parse -> trim 0.0.1

trim 0.0.1 has a ReDoS vulnerability

Please let me know if you need more information about this.
Can you fix this vuln?

@nickiaconis
Copy link

I can add a little data from my experience: specifying "**/remark-parse": "^9.0.0", our in package.json did remove the trim 0.0.1 dependency, but caused all *.stories.mdx files—which were previously working—to generate an Unexpected default export without title: {"includeStories":[],"parameters":{"docs":{}}} error. So it seems there may be changes needed to accommodate bumping the major version of remark-parse. Hope this info proves useful!

@astreiten
Copy link

Exactly, @nickiaconis. I´m facing the same issue. Do you know if is there a plan to fix this in the next release?

@tyro-e
Copy link

tyro-e commented Apr 30, 2021

Following this.

I am experiencing the same warnings on my side.

@shilman
Copy link
Member

shilman commented Apr 30, 2021

We are on the most recent version of @mdx-js/loader. Any ideas short of forking it?

@BiancaArtola
Copy link
Author

@shilman I created an issue on @mdx-js/loader: mdx-js/mdx#1531
Let's see if they help us!

@sfc-gh-acourtney
Copy link

There was a followup comments to the issue @BiancaArtola created: mdx-js/mdx#1531 (comment)
Apparently this is fixed in MDX 2 (currently in beta).
Thanks for looking into this, @shilman !

@shilman
Copy link
Member

shilman commented May 6, 2021

Nice @sfc-gh-acourtney. I can't wait for MDX2 -- the original release target was last July so ... 😰

@RyanMacBern
Copy link

The fix for me was to add to my package.json:

"resolutions": {
  "**/trim": "^1.0.0"
}

@AElmoznino
Copy link

@RyanMacBern What version of npm are you using? Doesn't work for me 😢

@ljones-i-nexus
Copy link

ljones-i-nexus commented May 12, 2021

@AElmoznino I believe "resolutions" is a Yarn feature. Happy to be corrected but I think the "has workaround" is only for Yarn users?

@AElmoznino
Copy link

@AElmoznino I believe "resolutions" is a Yarn feature. Happy to be corrected but I think the "has workaround" is only for Yarn users?

I got the abovee working with https://www.npmjs.com/package/npm-force-resolutions

@BiancaArtola
Copy link
Author

We used npm in our project and the solution with npm-force-resolution gives us a lot of trouble when building the code, so sadly this is not a solution for us.

david-crespo added a commit to oxidecomputer/console that referenced this issue May 20, 2021
fix comes from here:
storybookjs/storybook#14603 (comment)

it can be removed once storybook updates mdx and mdx updates remark-parse
@JJ
Copy link

JJ commented Jun 28, 2021

I can add a little data from my experience: specifying "**/remark-parse": "^9.0.0", our in package.json did remove the trim 0.0.1 dependency, but caused all *.stories.mdx files—which were previously working—to generate an Unexpected default export without title: {"includeStories":[],"parameters":{"docs":{}}} error. So it seems there may be changes needed to accommodate bumping the major version of remark-parse. Hope this info proves useful!

That does not work if you specify it in package.json either. The old version is still installed.

@JJ
Copy link

JJ commented Jun 28, 2021

We used npm in our project and the solution with npm-force-resolution gives us a lot of trouble when building the code, so sadly this is not a solution for us.

Plus adding another dependency just to solve a vulnerability introduced by one does not make a whole lot of sense. It's probably OK, if you already use it in your project (slash those troubles building the code, of course)

@deej-split
Copy link

@sfc-gh-acourtney fwiw, I raised this issue (inadvertently duplicately) at mdx-js/mdx#1597 . No idea when 2.0 of mdx-js/mdx@2 will come out... seems there's some dispute (2.x has been in the works for > 1 year). He does point out that there's another package one could look into using instead... https://github.com/wooorm/xdm. I haven't looked at how api-compatible it is, though.

@haposan06
Copy link

Using npm force resolutions later will give me this error when I run npm ls trim

npm ls trim
ex_proj@0.0.1 /ex_proj
├─┬ @storybook/addon-essentials@6.3.6
│ └─┬ @storybook/addon-docs@6.3.6
│   └─┬ @mdx-js/mdx@1.6.22
│     └─┬ remark-parse@8.0.3
│       └── UNMET DEPENDENCY trim@0.0.3 
└── UNMET DEPENDENCY trim@0.0.3 

npm ERR! missing: trim@0.0.3, required by ex_proj@0.0.1
npm ERR! extraneous: trim@0.0.3 /ex_proj/node_modules/trim

Is this safe to ignore?

nickpresta added a commit to Shopify/polaris-viz that referenced this issue Aug 17, 2021
The API for trim hasn't changed:

https://github.com/Trott/trim/blob/main/CHANGELOG.md

The issue stems from the version of remark used in Storybook. Issue open
here:

storybookjs/storybook#14603

And the change to remark to make this happen:

remarkjs/remark#782

TL;DR: Force it and move on with our lives
@ghost
Copy link

ghost commented Sep 9, 2021

Will this be fixed or need to use resolutions hack temporarily?

@j-jmnz
Copy link

j-jmnz commented Sep 30, 2021

Any news on fixing this vulnerability?

Zecat added a commit to Zecat/simple-vue-app that referenced this issue Mar 23, 2022
- Regular expression denial of service
- Regular Expression Denial of Service in trim

Those problems are somehow related to storybook and will probably be
fixed in future release. See storybookjs/storybook#14603
ovineq added a commit to equinor/esv-intersection that referenced this issue Mar 30, 2022
- Storybook related
- Prettier
- Typedoc

The remaining vulnerable packackes `trim` and `glob-parent` is related
to Storybook. There might come a fix for these once version 6.5 is
released: storybookjs/storybook#14603 (comment)
ovineq added a commit to equinor/esv-intersection that referenced this issue Apr 4, 2022
- Storybook related
- Prettier
- Typedoc

The remaining vulnerable packackes `trim` and `glob-parent` is related
to Storybook. There might come a fix for these once version 6.5 is
released: storybookjs/storybook#14603 (comment)
@valentinetech
Copy link

Same problem as well with glob-parent. Is there a hacky way to get around them?

@aaschlote
Copy link

Some news about this topic?

@shilman
Copy link
Member

shilman commented Apr 21, 2022

@aaschlote We can't upgrade the dependency until 7.0. If you are using Storybook Docs with MDX and you want to avoid this dependency, you can opt-in to MDX2 per https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#opt-in-mdx2-support

If you do that, you will still have the vulnerable dependency in your node_modules, it just won't be used. However, I believe that's also the case with MDX1 per discussions in that repo.

Hopefully 7.0 will be in alpha in a few weeks, at which point you'll be able to get a vulnerability-free dependency tree.

@IuliiaBondarieva
Copy link

storybook 6.4.22 introduces 19 high severity vulnerabilities with

  • glob-parent <5.1.2
  • trim <0.0.3

Is there any change planned at all??

@shilman
Copy link
Member

shilman commented May 17, 2022

@IuliiaBondarieva please see my comment above. The opt-in MDXv2 upgrade is available in 6.5. The full upgrade is a breaking change and will come in 7.0 (next release)

@SimenB
Copy link
Contributor

SimenB commented May 19, 2022

@shilman storybook 6.5 has introduced more vulnerabilities via the added x-default-browser dep in @storybook/core-server (#16844). Ends up loading trim-newlines@1.0.0, GHSA-7p7h-4mm5-852v.

Related, but module seems unmaintained: jakub-g/x-default-browser#8

@shilman
Copy link
Member

shilman commented May 19, 2022

@SimenB we have a fix here. will get it merged & update with a patch. thanks for the heads up! 🙏

#18157

jawinn added a commit to jawinn/game-of-go-react that referenced this issue May 19, 2022
Dependabot found dependency vulnerabilities were not fixed as they exist within dependencies for packages within package-lock. The ones found are related to Storybook and will be fixed in an upcoming version. See storybookjs/storybook#14603
@GuillaumeCisco
Copy link

Following: #18860
Can we update it in the recent alpha/beta version? i.e 7.x.x?

@shilman
Copy link
Member

shilman commented Aug 3, 2022

@GuillaumeCisco yes we can, it's on our list!

@pumano
Copy link

pumano commented Oct 9, 2022

@shilman could you please update @mdx-js/mdx to 2.0.x according to comment above? Looks like in 7 alpha.35 branch it's not updated, I tested it today.

  ┬ @storybook/addon-essentials@7.0.0-alpha.35
  └─┬ @storybook/addon-docs@7.0.0-alpha.35
    └─┬ @storybook/mdx1-csf@0.0.5-canary.13.9ce928a.0
      └─┬ @mdx-js/mdx@1.6.22
        └─┬ remark-parse@8.0.3
          └── trim@0.0.1

mattvanvoorst-contentful added a commit to contentful/template-marketing-webapp-nextjs that referenced this issue Oct 12, 2022
- Updated issues emerging from yarn audit, specifically for Storybook. Following storybookjs/storybook#14603 using yarn resolutions https://classic.yarnpkg.com/lang/en/docs/selective-version-resolutions/
@shilman shilman self-assigned this Oct 15, 2022
@shilman
Copy link
Member

shilman commented Oct 19, 2022

Yay!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.40 containing PR #19495 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Oct 19, 2022
@dreampulse
Copy link

dreampulse commented Oct 20, 2022

This is working for me:

"resolutions": {
    "**/trim": "^0.0.3"
  },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

No branches or pull requests