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

react-dev-utils uses a vulnerable version of immer as a dependency #10411

Closed
wclem4 opened this issue Jan 20, 2021 · 41 comments · Fixed by #10412
Closed

react-dev-utils uses a vulnerable version of immer as a dependency #10411

wclem4 opened this issue Jan 20, 2021 · 41 comments · Fixed by #10412

Comments

@wclem4
Copy link
Contributor

wclem4 commented Jan 20, 2021

Describe the bug

react-dev-utils package uses a vulnerable version (7.0.9) of immer as a dependency.

Here is the GitHub CVE (High Severity) notification for the vulnerability, and here is the commit that has fixed it in the Immer 8.0.1 release earlier today.

react-dev-utils should be updated to use version 8.0.1 of Immer.

wclem4 added a commit to wclem4/create-react-app that referenced this issue Jan 20, 2021
Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
wclem4 added a commit to wclem4/create-react-app that referenced this issue Jan 20, 2021
Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
@RDIL
Copy link
Contributor

RDIL commented Jan 21, 2021

Note: this will NOT make anybody's apps vulnerable.

This is a development-time only dependency.

Edit as of 2/3/2021 to clarify things for everyone: This is only used during the build phase of your app. It is not in the webpack bundle. Assuming your app doesn't directly use immer, e.g. you have manually put it in your dependencies, you are fine.

@jhockett
Copy link

Note: this will NOT make anybody's apps vulnerable. This is a development-time only dependency.

Is there a reason it's not a devDependency?

"dependencies": {
"@babel/code-frame": "7.10.4",
"address": "1.1.2",
"browserslist": "4.14.2",
"chalk": "2.4.2",
"cross-spawn": "7.0.3",
"detect-port-alt": "1.1.6",
"escape-string-regexp": "2.0.0",
"filesize": "6.1.0",
"find-up": "4.1.0",
"fork-ts-checker-webpack-plugin": "4.1.6",
"global-modules": "2.0.0",
"globby": "11.0.1",
"gzip-size": "5.1.1",
"immer": "8.0.1",
"is-root": "2.1.0",
"loader-utils": "2.0.0",
"open": "^7.0.2",
"pkg-up": "3.1.0",
"prompts": "2.4.0",
"react-error-overlay": "^6.0.8",
"recursive-readdir": "2.2.2",
"shell-quote": "1.7.2",
"strip-ansi": "6.0.0",
"text-table": "0.2.0"
},
"devDependencies": {
"cross-env": "^7.0.2",
"jest": "26.6.0"
},

@RDIL
Copy link
Contributor

RDIL commented Jan 23, 2021

Note: this will NOT make anybody's apps vulnerable. This is a development-time only dependency.

Is there a reason it's not a devDependency?

"dependencies": {
"@babel/code-frame": "7.10.4",
"address": "1.1.2",
"browserslist": "4.14.2",
"chalk": "2.4.2",
"cross-spawn": "7.0.3",
"detect-port-alt": "1.1.6",
"escape-string-regexp": "2.0.0",
"filesize": "6.1.0",
"find-up": "4.1.0",
"fork-ts-checker-webpack-plugin": "4.1.6",
"global-modules": "2.0.0",
"globby": "11.0.1",
"gzip-size": "5.1.1",
"immer": "8.0.1",
"is-root": "2.1.0",
"loader-utils": "2.0.0",
"open": "^7.0.2",
"pkg-up": "3.1.0",
"prompts": "2.4.0",
"react-error-overlay": "^6.0.8",
"recursive-readdir": "2.2.2",
"shell-quote": "1.7.2",
"strip-ansi": "6.0.0",
"text-table": "0.2.0"
},
"devDependencies": {
"cross-env": "^7.0.2",
"jest": "26.6.0"
},

What I meant is that it is essentially its only used during the build phase of your app, not at runtime. Sorry for phrasing it wrong.

@TianyuHou
Copy link

when can we expect to have a new release for this fix? this is a high security issue within our project

@repartay
Copy link

Bumping this request, we also have this as a high security vuln in our project and since immer has released the patch, would really appreciate a timely bump in your dependencies. Thank you!

@yec
Copy link

yec commented Feb 2, 2021

instead of making demands why don't you submit a patch

when can we expect to have a new release for this fix? this is a high security issue within our project

Bumping this request, we also have this as a high security vuln in our project and since immer has released the patch, would really appreciate a timely bump in your dependencies. Thank you!

@trivikr
Copy link

trivikr commented Feb 2, 2021

instead of making demands why don't you submit a patch

There's an open PR posted on the day this issue was created at #10412

@mrwensveen
Copy link

mrwensveen commented Feb 3, 2021

Thanks for looking at this, I really appreciate the hard work you people are doing!

Note: this will NOT make anybody's apps vulnerable. This is a development-time only dependency.

Is there a reason it's not a devDependency?

What I meant is that it is essentially its only used during the build phase of your app, not at runtime. Sorry for phrasing it wrong.

But doesn't that mean it should be a devDependency?

IMHO there is something seriously wrong with the security audit notifications. I've seen it dozens of times:

  1. npm warns about a vulnerability.
  2. developers want to be good citizens and submit a bug.
  3. package maintainers tell them there's nothing to worry about and leave the end users with alarms going off for weeks.

I think telling users to ignore security warnings is harmful. So either npm shouldn't warn about irrelevant vulnerabilities, or package maintainers should prioritize security vulnerabilities even if they are irrelevant.

@RDIL
Copy link
Contributor

RDIL commented Feb 3, 2021

But doesn't that mean it should be a devDependency?

If it WAS a devDependency for react-scripts, it wouldn't get installed during npm install or yarn install. It also can't be a dev dependency for every app, since the philosophy is having only one top-level dependency for the entire build process. Either way, this does not work as a dev dependency.

@adesnmi
Copy link

adesnmi commented Feb 3, 2021

IMHO there is something seriously wrong with the security audit notifications. I've seen it dozens of times:

Yup, at this point I’m inclined to turn it off which defeats the point of what should be a very useful feature. It’s annoying having the main repo page shout at me with nothing I can do about it.

@mrwensveen
Copy link

But doesn't that mean it should be a devDependency?

If it WAS a devDependency for react-scripts, it wouldn't get installed during npm install or yarn install. It also can't be a dev dependency for every app, since the philosophy is having only one top-level dependency for the entire build process. Either way, this does not work as a dev dependency.

I understand that react-scripts can't be a devDependency itself because of some issues (discussion). My best suggestion would be to split up the package into something like react-scripts and react-scripts-dev, with the latter as devDependency and depending on things like immer and other stuff that needs to be pulled in development/design time, but can be left out in production. This is akin to some Linux distributions, that have things like header files needed for developing with a library in a libXXX-dev package.

An other, more outlandish idea I had, but very much out of scope for this discussion, is to have the ability for packages to specify 'transient devDependencies', meaning that when you devDepend on a package specifying a transient devDependency, that dependency should be treated as a devDependency for your application or package as well.

@avbdev
Copy link

avbdev commented Feb 9, 2021

Even if it's dev dependency why do we have to stick with the oldest version of immer(1.10.0). Can we not bump up the version?

@RDIL
Copy link
Contributor

RDIL commented Feb 9, 2021

Even if it's dev dependency why do we have to stick with the oldest version of immer(1.10.0). Can we not bump up the version?

What version of react-dev-utils are you using @avbhardwaj?

@avbdev
Copy link

avbdev commented Feb 9, 2021

@RDIL : Currently I'm using v10.2.1

@RDIL
Copy link
Contributor

RDIL commented Feb 9, 2021

@RDIL : Currently I'm using v10.2.1

Are you sure? It appears that react-dev-utils is using a way newer build of immer than 1.10.0: https://github.com/facebook/create-react-app/blob/master/packages/react-dev-utils/package.json

@avbdev
Copy link

avbdev commented Feb 9, 2021

@RDIL : Thanks for the update 👍 , it seems like it has been bumped up in the v11.0.2 of react-dev-utils.
image

@juffel
Copy link

juffel commented Feb 10, 2021

Also the newer version of react-dev-tools@11.0.2 still depends on a vulnerable version of "immer": "7.0.9" 🤔
according to the CVE the vulnerability is patched in immer versions >8.0.1

@votemike
Copy link

Forgive my ignorance.
While immer might not be a problem in this package. The fixed dependency still stops immer being upgraded in other packages used in a project. No?

@RDIL
Copy link
Contributor

RDIL commented Feb 16, 2021

If your project uses a different major version of immer, your package manager should move this version directly into node_modules/react-dev-utils/node_modules/immer.

@KannugoPrithvi
Copy link

We are still getting this issue ever after upgrading to 11.0.2 in our project. Immer dependency is still at 7.0.9. Request the author to please upgrade the immer dependency.

@gaearon
Copy link
Contributor

gaearon commented Feb 22, 2021

So there's nothing wrong with my packages?

No, there’s nothing wrong. npm audit, when applied to build tools, is often useless, but unfortunately vulnerability advisories offer no way to tag whether a library’s usage in a build tool can lead to issues or not. In this case, it doesn’t.

For all who said this is not an important Bug: This a constraining bug which prevent me from see my code warnings on console.

The false positive report in this thread has no relation to the bug you are seeing. File a new issue with a reproducing project please.

@RadixSeven
Copy link

To get things moving using npm rather than yarn, you can use the StackOverflow answer here https://stackoverflow.com/a/48524488 to move it out of "requires" and into "dependencies" - where you can set the version. This will get overwritten whenever you use npm to add a new package, but you can add it back.

iansu pushed a commit that referenced this issue Feb 22, 2021
Resolves #10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
@chaim1221
Copy link

  "dependencies": {
#
    "react-dev-utils": "^11.0.2",
# snip
  },
  "resolutions": {
    "immer": "^8.0.1"
  },

Poof, you're unblocked.

# snip
success Saved lockfile.
Done in 8.39s.
celiyah@ce-y720-dev:~/Code/machete/react-ui$ yarn audit
yarn audit v1.22.5
0 vulnerabilities found - Packages audited: 1819
Done in 1.12s.

blackarctic added a commit to blackarctic/create-react-app that referenced this issue Apr 29, 2021
* Fix noFallthroughCasesInSwitch/jsx object is not extensible (facebook#9921)

Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>

* Add logo license to README

* Remove trailing space in reportWebVitals.ts (facebook#10040)

* docs: add React Testing Library as a library requiring jsdom (facebook#10052)

Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>

* Increase Workbox's maximumFileSizeToCacheInBytes (facebook#10048)

* Create FUNDING.yml

* replace inquirer with prompts (facebook#10083)

- remove `react-dev-utils/inquirer` public import

* Prepare 4.0.1 release

* Prepare 4.0.1 release

* Publish

 - cra-template-typescript@1.1.1
 - cra-template@1.1.1
 - create-react-app@4.0.1
 - react-dev-utils@11.0.1
 - react-scripts@4.0.1

* chore: bump web-vital dependency version (facebook#10143)

* chore: bump typescript version (facebook#10141)

Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>

* Add TypeScript 4.x as peerDependency to react-scripts(facebook#9964)

* remove chalk from formatWebpackMessages (facebook#10198)

* Upgrade @svgr/webpack to fix build error (facebook#10213)

Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>

* Improve vendor chunk names in development (facebook#9569)

* Update postcss packages (facebook#10003)

Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>

* Recovered some integration tests (facebook#10091)

* Upgrade sass-loader (facebook#9988)

* Move ESLint cache file into node_modules (facebook#9977)

Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>

* Revert "Update postcss packages" (facebook#10216)

This reverts commit 580ed5d.

* Remove references to Node 8 (facebook#10214)

* fix(react-scripts): add missing peer dependency react and update react-refresh-webpack-plugin (facebook#9872)

* Update using-the-public-folder.md (facebook#10314)

Some library --> Some libraries

* docs: add missing override options for Jest config (facebook#9473)

* Fix CI tests (facebook#10217)

* appTsConfig immutability handling by immer (facebook#10027)

Co-authored-by: mad-jose <joset@yeswearemad.com>

* Add support for new BUILD_PATH advanced configuration variable (facebook#8986)

* Add opt-out for eslint-webpack-plugin (facebook#10170)

* Prepare 4.0.2 release

* Publish

 - cra-template-typescript@1.1.2
 - cra-template@1.1.2
 - create-react-app@4.0.2
 - react-dev-utils@11.0.2
 - react-error-overlay@6.0.9
 - react-scripts@4.0.2

* tests: update test case to match the description (facebook#10384)

* Bump webpack-dev-server 3.11.0 -> 3.11.1 (facebook#10312)

Resolves facebook#10084 security vulnerability in websocket-driver library version 0.5.6, imported transitively by sockjs

* Upgrade eslint-webpack-plugin to fix opt-out flag (facebook#10590)

* update immer to 8.0.1 to address vulnerability (facebook#10412)

Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.

* Prepare 4.0.3 release

* Update CHANGELOG

* Publish

 - create-react-app@4.0.3
 - react-dev-utils@11.0.3
 - react-scripts@4.0.3

Co-authored-by: Ryota Murakami <dojce1048@gmail.com>
Co-authored-by: Konstantin Simeonov <kon.simeonov@protonmail.com>
Co-authored-by: Ian Sutherland <ian@iansutherland.ca>
Co-authored-by: sho90 <aznecosann@gmail.com>
Co-authored-by: Anyul Rivas <anyulled@gmail.com>
Co-authored-by: Ian Schmitz <ianschmitz@gmail.com>
Co-authored-by: Jeffrey Posnick <jeffy@google.com>
Co-authored-by: Evan Bacon <baconbrix@gmail.com>
Co-authored-by: Sahil Purav <sahil5684@gmail.com>
Co-authored-by: Hakjoon Sim <trainto@gmail.com>
Co-authored-by: Chris Shepherd <chris@chrisshepherd.me>
Co-authored-by: Jason Williams <936006+jasonwilliams@users.noreply.github.com>
Co-authored-by: Jabran Rafique⚡️ <jabranr@users.noreply.github.com>
Co-authored-by: John Ruble <johnruble@gmail.com>
Co-authored-by: Morten N.O. Nørgaard Henriksen <morten.n.o.henriksen@icloud.com>
Co-authored-by: Sergey Makarov <serega.s.makar@gmail.com>
Co-authored-by: EhsanKhaki <ehsankhfr@gmail.com>
Co-authored-by: Kristoffer K <merceyz@users.noreply.github.com>
Co-authored-by: Aviv Hadar <Avivhdr@gmail.com>
Co-authored-by: Tobias Büschel <13087421+tobiasbueschel@users.noreply.github.com>
Co-authored-by: mad-jose <44253495+josezone@users.noreply.github.com>
Co-authored-by: mad-jose <joset@yeswearemad.com>
Co-authored-by: Andrew Hyndman <ajhyndman@hotmail.com>
Co-authored-by: Brody McKee <mrmckeb@users.noreply.github.com>
Co-authored-by: James George <jamesgeorge998001@gmail.com>
Co-authored-by: Dion Woolley <woolley.dion@gmail.com>
Co-authored-by: Walker Clem <51654951+wclem4@users.noreply.github.com>
wombleton pushed a commit to AurorNZ/create-react-app that referenced this issue Jun 1, 2021
Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
westwood846 pushed a commit to glints-dev/glints-aries that referenced this issue Sep 9, 2021
This is following up on a vulnerability reported here:
https://github.com/glints-dev/glints-aries/security/dependabot/yarn.lock/immer/open

See the related discussion here:
facebook/create-react-app#10411

From what I found online, this issue is quite irrelevant:
facebook/create-react-app#10411 (comment)
westwood846 pushed a commit to glints-dev/glints-aries that referenced this issue Sep 9, 2021
This is following up on a vulnerability reported here:
https://github.com/glints-dev/glints-aries/security/dependabot/yarn.lock/immer/open

See the related discussion here:
facebook/create-react-app#10411

From what I found online, this issue is quite irrelevant:
facebook/create-react-app#10411 (comment)
@cmacdonnacha
Copy link

  "dependencies": {
#
    "react-dev-utils": "^11.0.2",
# snip
  },
  "resolutions": {
    "immer": "^8.0.1"
  },

Poof, you're unblocked.

# snip
success Saved lockfile.
Done in 8.39s.
celiyah@ce-y720-dev:~/Code/machete/react-ui$ yarn audit
yarn audit v1.22.5
0 vulnerabilities found - Packages audited: 1819
Done in 1.12s.

Is there any way to do this with NPM?

@gian-luca-weston
Copy link

Is there any way to do this with NPM?

Using the npm-force-resolution package , you can change the version of the sub-dependency within npm
https://www.npmjs.com/package/npm-force-resolutions

@cmacdonnacha
Copy link

cmacdonnacha commented Oct 21, 2021

Yep, I'm doing that now with all 5 vulbernable packages:

  "resolutions": {
    "ansi-regex": "5.0.1",
    "nth-check": "2.0.1",
    "set-value": "4.0.1",
    "immer": "9.0.6",
    "ansi-html": "https://registry.npmjs.org/ansi-html-community/-/ansi-html-community-0.0.8.tgz"
  },

@jacraven
Copy link

jacraven commented Dec 10, 2021

Thanks for the help! I just went through the same issue & this was quite helpful.

An updated list of resolutions to patch up current vulnerabilities as of December 2021 was as follows:

"resolutions": {
    "immer": "9.0.7",
    "ansi-html": "https://registry.yarnpkg.com/ansi-html-community/-/ansi-html-community-0.0.8.tgz",
    "ansi-regex": "5.0.1",
    "nth-check": "2.0.1",
    "glob-parent": "6.0.1",
    "browserslist": "4.18.1"
  }

@veller
Copy link

veller commented Jan 12, 2022

I tried using both resolutions and overrides (from npm) but none worked. My application uses Nexus as a private npm registry so that caused an error when using npm-force-resolutions.

Just to make sure I'm doing it correctly, @cmacdonnacha and @jacraven, you guys are just adding the resolutions' object to package.json and running npm i afterwards?

@RDIL
Copy link
Contributor

RDIL commented Jan 12, 2022

@veller please update your react scripts version if possible, but also keep in mind this isn't going to harm your project, and can safely be ignored.

abhiisheek pushed a commit to abhiisheek/create-react-app that referenced this issue May 19, 2023
Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
abhiisheek pushed a commit to abhiisheek/create-react-app that referenced this issue May 24, 2023
Resolves facebook#10411

Bumps immer version to 8.0.1 to address the prototype pollution
vulnerability with the current 7.0.9 version.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.