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

Upgrade loader-utils to 2.0.4 in storysource and source-loader #19891

Merged
merged 2 commits into from
Nov 19, 2022

Conversation

mukundkatpatal
Copy link

@mukundkatpatal mukundkatpatal commented Nov 18, 2022

… to resolve a critical security issue found by npm audit
I just upgraded the versions of the loader utils in source-loader and storysource to resolve the security finding from the npm audit

Issue: critical security vulnerability in the loader-utils

What I did

Upgraded the loader-utils to 2.0.4 in storysource and source-loader

How to test

loader-utils 2.0.0 - 2.0.3 || 3.0.0 - 3.2.0
Severity: critical
Prototype pollution in webpack loader-utils - GHSA-76p3-8jx3-jpfq
loader-utils is vulnerable to Regular Expression Denial of Service (ReDoS) via url variable - GHSA-3rfm-jhwj-7488
loader-utils is vulnerable to Regular Expression Denial of Service (ReDoS) via url variable - GHSA-3rfm-jhwj-7488
loader-utils is vulnerable to Regular Expression Denial of Service (ReDoS) - GHSA-hhq3-ff78-jv3g
loader-utils is vulnerable to Regular Expression Denial of Service (ReDoS) - GHSA-hhq3-ff78-jv3g
fix available via npm audit fix
node_modules/@storybook/addon-storysource/node_modules/loader-utils
node_modules/@storybook/angular/node_modules/loader-utils
node_modules/@storybook/builder-webpack5/node_modules/loader-utils
node_modules/@storybook/manager-webpack5/node_modules/loader-utils
node_modules/adjust-sourcemap-loader/node_modules/loader-utils
node_modules/babel-loader/node_modules/loader-utils
node_modules/loader-utils
node_modules/resolve-url-loader/node_modules/loader-utils
node_modules/style-loader/node_modules/loader-utils
node_modules/ts-loader/node_modules/loader-utils

If your answer is yes to any of these, please make sure to include it in your PR.

To @maintainers, I would like this to be patched for 6.5.13 versions.
Although this issue might have been fixed for 7-next, since there is no stable version 7, I need to have 6.5.14 as a stable version.

… to resolve a critical security issue found by npm audit
Copy link
Contributor

@JReinhold JReinhold left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for being on top of this!
We should change this to version 2.0.4 instead of 2.0.3, and run yarn install in both packages to ensure yarn.locks are also being re-generated.

@ndelangen ndelangen self-assigned this Nov 18, 2022
@ndelangen
Copy link
Member

@shilman this should be patched to 6.5 ?

@shilman shilman changed the title Build: Upgrade loader-utils to 2.0.3 in storysource and source-loader… Upgrade loader-utils to 2.0.4 in storysource and source-loader Nov 19, 2022
@shilman shilman merged commit a8a537d into storybookjs:main-prerelease Nov 19, 2022
@shilman
Copy link
Member

shilman commented Nov 19, 2022

@ndelangen this is already targeting the main-prerelease branch

@bnussman-akamai
Copy link

bnussman-akamai commented Dec 13, 2022

@storybook/[email protected] uses [email protected] and [email protected] which is many years old which causes [email protected] to be installed. For the average storybook user, this didn't resolve the loader-utils vulnerabilities in Storybook.

@storybook/builder-webpack4 needs to update its css-loader version or storybook needs to stop shipping with webpack

Copy link
Member

shilman commented Dec 15, 2022

@bnussman-akamai there are lots of security issues with webpack4. if you're concerned, please upgrade to the latest prerelease which gets rid of webpack4 entirely and handles all known npm audit issues.

npx sb@next upgrade --prerelease

WIP migration guide here https://chromatic-ui.notion.site/Storybook-7-migration-guide-dbf41fa347304eb2a5e9c69b34503937

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

Successfully merging this pull request may close these issues.

5 participants