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

CVE-2021-42740 in shell-quote #19785

Closed
mrbusche opened this issue Jan 20, 2022 · 7 comments
Closed

CVE-2021-42740 in shell-quote #19785

mrbusche opened this issue Jan 20, 2022 · 7 comments
Labels
stale no activity on this issue for a long period type: bug type: dependencies type: security 🔐 Security related

Comments

@mrbusche
Copy link

Current behavior

CVE-2021-42740 is a critical CVE in shell-quote version 1.7.2 that is resolved in 1.7.3

The shell-quote package before 1.7.3 for Node.js allows command injection. An attacker can inject unescaped shell metacharacters through a regex designed to support Windows drive letters. If the output of this package is passed to a real shell as a quoted argument to a command with exec(), an attacker can inject arbitrary commands. This is because the Windows drive letter regex character class is {A-z] instead of the correct {A-Za-z]. Several shell metacharacters exist in the space between capital letter Z and lower case letter a, such as the backtick character.

Desired behavior

cypress uses shell-quote 1.7.3

Test code to reproduce

It's a publicly visible CVE

Cypress Version

9.3.1

Other

No response

@tbiethman
Copy link
Contributor

We get shell-quote@1.7.2 from a dependency on react-dev-utils@11.x, which is a transitive dependency coming from a few others. react-dev-utils@12 has bumped the version to ^1.7.3, so we need to trace back up to us and see what can be bumped.

=> Found "react-dev-utils@11.0.4"
info Has been hoisted to "react-dev-utils"
info Reasons this module exists
   - "workspace-aggregator-fea014e7-9a83-4e2a-a6f8-eef14214b3d2" depends on it
   - Hoisted from "_project_#@cypress#design-system#@storybook#react#react-dev-utils"
   - Hoisted from "_project_#@cypress#design-system#@storybook#addon-essentials#@storybook#addon-docs#@storybook#builder-webpack4#react-dev-utils"
=> Found "@cypress/react#react-dev-utils@10.2.1"
info Reasons this module exists
   - "_project_#@cypress#react#react-scripts" depends on it
   - Hoisted from "_project_#@cypress#react#react-scripts#react-dev-utils"
=> Found "react-scripts#react-dev-utils@9.1.0"

@tbiethman
Copy link
Contributor

@mrbusche We would welcome a PR that would bump the relevant development dependencies in our @cypress/react and @cypress/design-system packages. The cypress package itself does not bundle a shell-quote@1.7.2 dependency and should not be impacted.

@cypress-bot cypress-bot bot added the stage: needs review The PR code is done & tested, needs review label Jan 29, 2022
@mrbusche
Copy link
Author

mrbusche commented Jan 29, 2022

Here are all the instances, via npm ls shell-quote, most package upgrades require breaking changes, especially react-scripts

cypress@9.3.1 C:\code\cypress
├─┬ @cypress/angular@0.0.0-development -> .\npm\angular
│ └─┬ @cypress/code-coverage@3.9.12
│   └─┬ @cypress/browserify-preprocessor@3.0.2
│     ├─┬ browserify@16.5.2
│     │ └── shell-quote@1.7.2 deduped
│     └─┬ watchify@4.0.0
│       ├─┬ browserify@17.0.0
│       │ └── shell-quote@1.7.2 deduped
│       └─┬ outpipe@1.1.1
│         └── shell-quote@1.7.2 deduped
├─┬ @cypress/react@0.0.0-development -> .\npm\react
│ ├─┬ @cypress/code-coverage@3.9.4
│ │ └─┬ @cypress/browserify-preprocessor@3.0.1
│ │   └─┬ browserify@16.2.3
│ │     └── shell-quote@1.7.2 deduped
│ ├─┬ next@10.2.3
│ │ └─┬ @next/react-dev-overlay@10.2.3
│ │   └── shell-quote@1.7.2 deduped
│ └─┬ react-scripts@3.4.1
│   └─┬ react-dev-utils@10.2.1
│     └── shell-quote@1.7.2 deduped
├─┬ @cypress/webpack-preprocessor@0.0.0-development -> .\npm\webpack-preprocessor
│ └─┬ react-scripts@3.2.0
│   └─┬ react-dev-utils@9.1.0
│     └── shell-quote@1.7.2 deduped
├─┬ @packages/example@0.0.0-development -> .\packages\example
│ └─┬ cypress-example-kitchensink@1.15.2
│   └─┬ npm-run-all@4.1.5
│     └── shell-quote@1.7.2 deduped
└─┬ @packages/server@0.0.0-development -> .\packages\server
  └─┬ launch-editor@2.3.0
    └── shell-quote@1.7.2

I was able to overwrite the resolutions by adding

"resolutions": {
    "shell-quote": "1.7.3"
}

and removing shell-quote from yarn.lock and doing a yarn install. react-dev-utils is pinned to 1.7.2 but since it's a dev dependency that shouldn't make it's way into the final image.

@mrbusche
Copy link
Author

My PR was rejected because I didn't make changes to upgrade to the latest major version, which would require a lot more time than I have to contribute. I commented on an issue for react-scripts to see if they'd accept a patch to version 3, but I don't feel confident - facebook/create-react-app#11608

@mrbusche
Copy link
Author

mrbusche commented Feb 9, 2022

Another solution may be to move react-scripts to devDependencies everywhere. It's currently listed as a dependency under find-webpack and craco. Then when buidling cypress build as a production build vs a dev/test bundle that includes devDependencies.

@cypress-bot cypress-bot bot added stage: backlog and removed stage: needs review The PR code is done & tested, needs review labels Apr 29, 2022
@cypress-app-bot
Copy link
Collaborator

This issue has not had any activity in 180 days. Cypress evolves quickly and the reported behavior should be tested on the latest version of Cypress to verify the behavior is still occurring. It will be closed in 14 days if no updates are provided.

@cypress-app-bot cypress-app-bot added the stale no activity on this issue for a long period label May 15, 2023
@cypress-app-bot
Copy link
Collaborator

This issue has been closed due to inactivity.

@cypress-app-bot cypress-app-bot closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale no activity on this issue for a long period type: bug type: dependencies type: security 🔐 Security related
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants