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

Deps: upgrade react-dev-utils to get newer immer #14015

Merged
merged 1 commit into from Feb 23, 2021

Conversation

phated
Copy link
Contributor

@phated phated commented Feb 22, 2021

Issue: #13961

What I did

I upgraded react-dev-utils to the new patch version that includes the immer without prototype pollution.

How to test

  • Is this testable with Jest or Chromatic screenshots? ❌
  • Does this need a new example in the kitchen sink apps? ❌
  • Does this need an update to the documentation? ❌

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

@sandrogomez
Copy link

👍

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

LGTM thanks! 🙏

@shilman shilman merged commit c762a15 into storybookjs:next Feb 23, 2021
@shilman shilman added patch:yes Bugfix & documentation PR that need to be picked to main branch patch:done Patch/release PRs already cherry-picked to main/release branch labels Feb 23, 2021
shilman added a commit that referenced this pull request Feb 24, 2021
Deps: upgrade react-dev-utils to get newer immer
This was referenced Mar 7, 2021
This was referenced Mar 14, 2021
@timbomckay
Copy link

We're on Storybook v6.3.8 and getting immer as a critical vulnerability. Does this fix that? Has this been released or is it slated for v6.4 or v7?

@shilman
Copy link
Member

shilman commented Sep 17, 2021

@timbomckay this should be in 6.3.x ... 6.2.x even!

@timbomckay
Copy link

We just wiped our package-lock and reinstalled. Here's the versions we're on:

  • @storybook/addon-essentials: ^6.3.8
  • @storybook/addon-knobs: ^6.3.1
  • @storybook/theming: ^6.3.8
  • @storybook/vue: ^6.3.8

When we run npm audit we get the following 4 criticals:

Critical        This affects the package immer before 9.0.6. A type           
                confusion vulnerability can lead to a bypass of               
                CVE-2020-28477 when the user-provided keys used in the path   
                parameter are arrays. In particular, this bypass is possible  
                because the condition (p === "__proto__" || p ===             
                "constructor") in applyPatches_ returns false if p is         
                ['__proto__'] (or ['constructor']). The === operator (strict  
                equality operator) returns false if the operands have         
                different type.                                               
Package         immer                                                         
Patched in      9.0.6                                                         
Dependency of   @storybook/vue [dev]                                          
Path            @storybook/vue > @storybook/core > @storybook/core-server >   
                @storybook/builder-webpack4 > react-dev-utils > immer         
More info       https://nodesecurity.io/advisories/184711                     
Critical        This affects the package immer before 9.0.6. A type           
                confusion vulnerability can lead to a bypass of               
                CVE-2020-28477 when the user-provided keys used in the path   
                parameter are arrays. In particular, this bypass is possible  
                because the condition (p === "__proto__" || p ===             
                "constructor") in applyPatches_ returns false if p is         
                ['__proto__'] (or ['constructor']). The === operator (strict  
                equality operator) returns false if the operands have         
                different type.                                               
Package         immer                                                         
Patched in      9.0.6                                                         
Dependency of   @storybook/addon-essentials [dev]                             
Path            @storybook/addon-essentials > @storybook/addon-docs >         
                @storybook/core > @storybook/core-server >                    
                @storybook/builder-webpack4 > react-dev-utils > immer         
More info       https://nodesecurity.io/advisories/184711                     
Critical        immer is vulnerable to Improperly Controlled Modification of  
                Object Prototype Attributes ('Prototype Pollution')           
Package         immer                                                         
Patched in      9.0.6                                                         
Dependency of   @storybook/vue [dev]                                          
Path            @storybook/vue > @storybook/core > @storybook/core-server >   
                @storybook/builder-webpack4 > react-dev-utils > immer         
More info       https://nodesecurity.io/advisories/184353                     
Critical        immer is vulnerable to Improperly Controlled Modification of  
                Object Prototype Attributes ('Prototype Pollution')           
Package         immer                                                         
Patched in      9.0.6                                                         
Dependency of   @storybook/addon-essentials [dev]                             
Path            @storybook/addon-essentials > @storybook/addon-docs >         
                @storybook/core > @storybook/core-server >                    
                @storybook/builder-webpack4 > react-dev-utils > immer         
More info       https://nodesecurity.io/advisories/184353       

Which it does appear that react-dev-tools installed latest of version 11.0.4 which still has a hard dependency of "immer": "8.0.4". So it seems like this isn't a matter of Storybook updating a dependency but react-dev-tools. Found a couple issues reported on their github.

Alright, well thanks. Should've dug deeper into this before commenting, as well as posting on the referenced issue haha. Anyways, sorry about that and thanks for the quick reply.

@shilman shilman added this to the 6.4 PRs milestone Sep 22, 2021
@literalpie
Copy link
Contributor

My understanding is that the update was merged into react-dev-utils, but it hasn't been a new version of 11.x released. Only 12.x prereleases (see here).

This comment suggests that this is not as urgent as the audit suggests, since it is only an issue in a dev tool, which I guess isn't used in any way that exposes the vulnerability to untrusted code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants