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-2024-47764 vulnerability from the cookie package #2308

Closed
4 tasks done
jamescrowley opened this issue Oct 9, 2024 · 14 comments · Fixed by #2312
Closed
4 tasks done

CVE-2024-47764 vulnerability from the cookie package #2308

jamescrowley opened this issue Oct 9, 2024 · 14 comments · Fixed by #2312
Assignees
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser

Comments

@jamescrowley
Copy link

jamescrowley commented Oct 9, 2024

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 18 or higher

Browsers

No response

Reproduction repository

https://github.com/boxwise/boxtribute

Reproduction steps

The latest msw release (<= 2.4.9) has a transient dependency on several versions of the cookie library that are < 7.0.0, which has a vulnerability (GHSA-pxg6-pf52-xh8x).

Current behavior

You are referencing it via

While it's unlikely to impact msw given your use-case, it will force the dependency to a lower version for others that use it in production-facing code.

Expected behavior

No security warnings

@jamescrowley jamescrowley added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser labels Oct 9, 2024
@kettanaito
Copy link
Member

Hi, @jamescrowley. Thanks for raising this.

So the issue is that @bundled-es-modules/cookie hasn't updated the cookie version in a while.

wrapper appears stale - may be possible to just reference cookie directly now?

Does it finally ship as proper ESM? I will have to take a look, it would be great to drop the wrapper if that's the case.

@kettanaito kettanaito self-assigned this Oct 9, 2024
@jamescrowley
Copy link
Author

jamescrowley commented Oct 9, 2024

@kettanaito they've decided to remain CJS only, but it appears to work fine with your set up.

I just did a quick PoC here, removing the bundled es module. Appears to lint & build fine (but haven't checked further than that)

I only upgraded to 0.7.2 as 1.0.0 has a breaking signature change.

@kettanaito
Copy link
Member

Thanks! I gave it a try in #2321, and it looks good so far. Will wait for the CI results and then proceed with publishing it.

@kettanaito
Copy link
Member

kettanaito commented Oct 10, 2024

Yeah, so cookie being CJS-only breaks our ESM consumers (ref). We cannot use it as-is, it has to be ESM. I will open a PR against @bundled-es-modules to bump the cookie version instead.

The effort already exists: bundled-es-modules/cookie#3

@domon-envato
Copy link

Hi, 👋

Thanks for the hard work on the fix! 🙏

Will the cookie vulnerability also be fixed in msw v1?

I couldn't find any information on the backporting policy. Apologies if this is not the right place to ask.

Thanks again!

@kettanaito
Copy link
Member

Hi, @domon-envato. This is absolutely the right place to ask.

We support the backports for security vulnerabilities based on the time availability. We are also welcoming contributors to open pull requests to ship those backports, if the matter is time-pressing.

MSW v1 doesn't depend on @bundled-es-modules/cookie but on cookie instead. If that's a non-breaking version bump (which, I hope it is on their side), provisioning a backport shouldn't be a problem.

Backports also release automatically to NPM as soon as they are merged under the backport tag.

I need to add a decision document about backports 👍

Would you be interesting in seeing this one through? I will share a more detailed instruction in the decision document later today, hopefully.

@TannerS
Copy link

TannerS commented Oct 28, 2024

Hello, what is the status of this? thank you

@kettanaito
Copy link
Member

@TannerS, you can find the latest status on the PR, always: #2312 (comment). Looks like we are blocked by the dependency where the fix has been merged but wasn't yet released. I tried moving this in-house but don't have the capacity right now (and that's likely not a good idea anyway). The sad reality of not publishing ESM in 2024.

@TannerS
Copy link

TannerS commented Oct 30, 2024

Thanks for the update, i am still learning but interested in understanding what you mean by The sad reality of not publishing ESM in 2024.?

:D

@Pewtro
Copy link

Pewtro commented Nov 4, 2024

Could it make sense to consider swapping from cooke (and thus @bundled-es-modules/cookie) to something like https://github.com/unjs/cookie-es ? It has the advantage of being maintained by the larger unjs org

@curtdept
Copy link

curtdept commented Nov 5, 2024

At this point, its what like 5 functions and maybe 100 lines of code? I would just make it native to msw and cut the dependency. The updation lag time of this with a CVE is not good. Node itself is moving faster.

@Pewtro
Copy link

Pewtro commented Nov 7, 2024

2.0.1 of @bundled-es-modules/cookie has been released, so people can run npm audit fix or similiar commands to update their package-lock files to resolve this vulnerability (if they don't want to manually update it).

@kettanaito
Copy link
Member

@curtdept, I wouldn't replicate packages, especially those that are getting CVE reports. The issue is not in cookie. The issue is in how it's distributed and how we expect to rely on it in MSW.

I've got some good feedback from the Node.js folks behind cookie. Will explore how to improve this dependency chain in the future.

@kettanaito kettanaito changed the title vuln: cookie package CVE-2024-47764 vulnerability from the cookie package Nov 7, 2024
@kettanaito
Copy link
Member

Released: v2.6.2 🎉

This has been released in v2.6.2!

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.


Predictable release automation by @ossjs/release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:browser Related to MSW running in a browser
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants