search

mswjs / msw

Industry standard API mocking for JavaScript.
https://mswjs.io
MIT License
15.97k stars 519 forks source link

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

Closed jamescrowley closed 2 weeks ago

jamescrowley commented 1 month ago

Prerequisites

Environment check

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 (https://github.com/advisories/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

kettanaito commented 1 month ago

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.

jamescrowley commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

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: https://github.com/bundled-es-modules/cookie/pull/3

domon-envato commented 1 month ago

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 commented 1 month ago

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 commented 3 weeks ago

Hello, what is the status of this? thank you

kettanaito commented 3 weeks ago

@TannerS, you can find the latest status on the PR, always: https://github.com/mswjs/msw/pull/2312#issuecomment-2408907675. 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 commented 3 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

@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 commented 2 weeks ago

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.

SamMousa commented 1 week ago

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.

Would you consider dropping it for a different package? https://github.com/unjs/cookie-es

This is a fork that has been maintained for several years, has ~1 million weekly downloads vs the wrappers ~2 million.

Unfortunately the jshttp/cookie seems to be in a committed exclusive relationship with cjs. Found it in here: https://github.com/jshttp/cookie/issues/152#issuecomment-2390659837

curtdept commented 1 week ago

Would you consider dropping it for a different package? https://github.com/unjs/cookie-es

@kettanaito As a rebuttal, in a way, the issue is cookie because it requires this heavily lagged, generated wrapper due to lack of esm support.

The above seems to be a worthy maintained alternative.

kettanaito commented 4 days ago

Sounds good to me. Pull requests are welcome!