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 Issue reported for tough-cookie, latest version of @cypress/request is dependent of tough-cookie@2.5.0 #27261

Closed
Kathuria opened this issue Jul 10, 2023 · 21 comments · Fixed by #27515
Assignees

Comments

@Kathuria
Copy link

Current behavior

It's working fine but having vulnerability issue on package @cypress/request or others consuming tough-cookie@2.5.0 which is <4.1.3

Desired behavior

Upgrade tough-cookie version to tough-cookie@4.1.3 for all packages having dependencies on this

Here is an example from latest tag, yarn-lock

"@cypress/request@^2.88.11":
version "2.88.11"
resolved "https://registry.yarnpkg.com/@cypress/request/-/request-2.88.11.tgz#5a4c7399bc2d7e7ed56e92ce5acb620c8b187047"
integrity sha512-M83/wfQ1EkspjkE2lNWNV5ui2Cv7UCv1swW1DqljahbzLVWltcsexQh8jYtuS/vzFXP+HySntGM83ZXA9fn17w==
dependencies:
aws-sign2 "~0.7.0"
aws4 "^1.8.0"
caseless "~0.12.0"
combined-stream "~1.0.6"
extend "~3.0.2"
forever-agent "~0.6.1"
form-data "~2.3.2"
http-signature "~1.3.6"
is-typedarray "~1.0.0"
isstream "~0.1.2"
json-stringify-safe "~5.0.1"
mime-types "~2.1.19"
performance-now "^2.1.0"
qs "~6.10.3"
safe-buffer "^5.1.2"
tough-cookie "~2.5.0"
tunnel-agent "^0.6.0"
uuid "^8.3.2"

Test code to reproduce

NA

Cypress Version

12.17.1

Node version

v16.15.0

Operating System

windows

Debug Logs

NA

Other

No response

@MikeMcC399

This comment was marked as outdated.

@BreakBB
Copy link

BreakBB commented Jul 11, 2023

@MikeMcC399 it should be noted that @cypress/request is affected by this: cypress-io/request#31

So I am not sure if the PR you linked is sufficient for this issue.

#27005 also required the request package to be updated.

@MikeMcC399
Copy link
Contributor

@BreakBB

So I am not sure if the PR you linked is sufficient for this issue.

You're right. npm audit shows the following for cypress@12.17.1 (current latest version):

$ npm audit
# npm audit report

tough-cookie  <4.1.3
Severity: moderate
tough-cookie Prototype Pollution vulnerability - https://github.com/advisories/GHSA-72xf-g2v4-qvf3
fix available via `npm audit fix --force`
Will install cypress@4.2.0, which is a breaking change
node_modules/tough-cookie
  @cypress/request  *
  Depends on vulnerable versions of tough-cookie
  node_modules/@cypress/request
    cypress  >=4.3.0
    Depends on vulnerable versions of @cypress/request
    node_modules/cypress

3 moderate severity vulnerabilities

To address all issues (including breaking changes), run:
  npm audit fix --force

@BreakBB
Copy link

BreakBB commented Jul 20, 2023

Any news or estimate on this?

It seems like a very small change without many risks and the PRs are already there 🤔

I am not sure if this is something worth reporting as Security Issue.

EDIT: A security issue as mail has been sent.

@TheLandolorien
Copy link

NOTE: This is a temporarily work around with npm until tough-cookie dependency version bump is merged. Add the following to package.json:

"overrides": {
  "tough-cookie": "^4.1.3"
}

Source: cypress-io/request#32 (comment)

ismarslomic added a commit to ismarslomic/MMM-Hello-World-Ts that referenced this issue Jul 31, 2023
@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Aug 1, 2023

Edit: Now v2.88.12 has been published.

@nicogominet
Copy link

As v12.17.3 has failed, can we include @cypress/request v2.88.12 in it? Unless you can tag v12.17.4 right after?

Thanks.

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Aug 1, 2023

@nicogominet

Can you try npm audit fix now?

Even a clean install of an earlier version like Cypress 10.0.0 will pick up the latest version of @cypress/request now.

npx cypress version
Cypress package version: 12.17.3
Cypress binary version: 12.17.3
Electron version: 21.0.0
Bundled Node version:
16.16.0

npm audit
found 0 vulnerabilities

Alternatively:

npm update @cypress/request

should also fix things.

@G-Rath
Copy link

G-Rath commented Aug 1, 2023

@MikeMcC399 note that @cypress/request is still vulnerable to GHSA-p8p7-x288-28g6, it just doesn't show on npm audit because the advisory range doesn't include the new version as vulnerable.

I've opened a PR to update the advisory after which it'll get flagged by dependabot and npm audit again

@MikeMcC399
Copy link
Contributor

@G-Rath

note that @cypress/request is still vulnerable to GHSA-p8p7-x288-28g6, it just doesn't show on npm audit because the advisory range doesn't include the new version as vulnerable.

Understood. This issue was just about tough-cookie.

@G-Rath
Copy link

G-Rath commented Aug 1, 2023

Yup no worries, I just didn't want to risk losing the attention on @cypress/request if the advisory doesn't get updated before this issue is closed :)

@MikeMcC399
Copy link
Contributor

@Kathuria

Is this fixed for you now or were you expecting that Cypress would force an update to the corrected version of @cypress/request, which would require bumping the dependency which is used to build the npm cypress module?

"@cypress/request": "^2.88.11",

@nicogominet
Copy link

nicogominet commented Aug 2, 2023

@MikeMcC399 I'm using yarn and npm audit fix equivalent did not fix it unfortunately, but a simple yarn upgrade did the trick.

@MikeMcC399
Copy link
Contributor

@nicogominet

I'm using yarn and npm audit fix equivalent did not fix it unfortunately, but a simple yarn upgrade did the trick.

Good to hear that you have the fix installed! Each of the different package managers has their own quirks!

I would welcome Cypress bumping the dependency definition (see #27261 (comment)) and I was hoping for some feedback from the original contributor @Kathuria and the issue assignee @cacieprins on this topic. Without this bump there is no clear universal way to ensure that the dependency gets updated for all the different flavors of package managers. If you already have Cypress installed, updating to a newer Cypress version will not normally update the version of @cypress/request to 2.88.12 because the semver is already satisfied by @cypress/request@2.88.11.

@Kathuria
Copy link
Author

Kathuria commented Aug 2, 2023

@Kathuria

Is this fixed for you now or were you expecting that Cypress would force an update to the corrected version of @cypress/request, which would require bumping the dependency which is used to build the npm cypress module?

"@cypress/request": "^2.88.11",

@MikeMcC399 : I will be waiting to get final version containing this vulnerability resolution. That would be proper fix going ahead in my project.

@Kathuria
Copy link
Author

Kathuria commented Aug 2, 2023

@nicogominet

I'm using yarn and npm audit fix equivalent did not fix it unfortunately, but a simple yarn upgrade did the trick.

Good to hear that you have the fix installed! Each of the different package managers has their own quirks!

I would welcome Cypress bumping the dependency definition (see #27261 (comment)) and I was hoping for some feedback from the original contributor @Kathuria and the issue assignee @cacieprins on this topic. Without this bump there is no clear universal way to ensure that the dependency gets updated for all the different flavors of package managers. If you already have Cypress installed, updating to a newer Cypress version will not normally update the version of @cypress/request to 2.88.12 because the semver is already satisfied by @cypress/request@2.88.11.

Agree, Dependencies upgrade would be expected fix for my requirement but open for suggestions also if any alternative way can work until the version is available through cypress patch/release.

@MikeMcC399
Copy link
Contributor

@Kathuria

if any alternative way can work until the version is available through cypress patch/release.

One alternative is to manually install @cypress/request@2.88.12.

@MikeMcC399
Copy link
Contributor

@G-Rath
Copy link

G-Rath commented Aug 3, 2023

You can also remove the dependency from the yarn.lock manually or using this script of mine, then running yarn install - that will allow yarn to re-resolve the version based on the constraints.

For completeness:

Package Manager Steps
npm <7 npm_remove_package_from_lock @cypress/request; npm install
npm >6 npm update @cypress/request
yarn yarn_remove_package_from_lock @cypress/request; yarn install
pnpm pnpm update @cypress/request

@MikeMcC399
Copy link
Contributor

MikeMcC399 commented Aug 10, 2023

Edit: Superseded by

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Aug 15, 2023

Released in 12.17.4.

This comment thread has been locked. If you are still experiencing this issue after upgrading to
Cypress v12.17.4, please open a new issue.

@cypress-bot cypress-bot bot locked as resolved and limited conversation to collaborators Aug 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.