kentcdodds / babel-plugin-macros

🎣 Allows you to build simple compile-time libraries
https://npm.im/babel-plugin-macros
MIT License
2.62k stars 135 forks source link

Depending on vulnerable version of cosmiconfig #192

Closed imnasnainaec closed 1 year ago

imnasnainaec commented 1 year ago

In package.json:

  "dependencies": {
    "cosmiconfig": "^7.0.0",

What you did: npm audit

What happened:

  cosmiconfig  6.0.0 - 7.1.0
  Depends on vulnerable versions of yaml

Problem description:

babel-plugin-macros depends on an older version of cosmicconfig which has a vulnerability.

Suggested solution:

Update cosmicconfig (at this time, v8.1.3 is available).

imnasnainaec commented 1 year ago

Related: #107 #127 #183

conartist6 commented 1 year ago

What are the breaking changes between cosmiconfig 7 and 8, and would any of them affect babel-plugin-macros?

Also I am not yet convinced that a real vulnerability exists. Which CVE are we talking about with yaml?

jrmhaig commented 1 year ago

Also I am not yet convinced that a real vulnerability exists.

I don't know the details, nor how reliable is this source, but here is what I found: https://huntr.dev/bounties/4b494e99-5a3e-40d9-8678-277f3060e96c/

rattkin commented 1 year ago

Also I am not yet convinced that a real vulnerability exists. Which CVE are we talking about with yaml?

Please not another "is this really a real vuln" conversation https://github.com/advisories/GHSA-f9xv-q969-pqx4

JulesFaucherre commented 1 year ago

Apparently a PR is already opened to solve this

masinger commented 1 year ago

Also I am not yet convinced that a real vulnerability exists. Which CVE are we talking about with yaml?

Please not another "is this really a real vuln" conversation GHSA-f9xv-q969-pqx4

But indeed it seems to be a false positive, as the maintainer seems to be trying to correct the affected versions to be 2.0.0-5. See https://huntr.dev/bounties/4b494e99-5a3e-40d9-8678-277f3060e96c/. And as far as I can tell, cosmiconfig v7 is still using the unaffected <2.0.0 version.

@admin How can I mark it so that the affected version range has a minimum of 2.0.0-5?

ricardo-passthrough commented 1 year ago

is a security update on cosmiconfig@7 be plausible? happy to start an issue there

ricardo-passthrough commented 1 year ago

a fix on the range will also remove any reports, I'll keep watching. I don't think there's a rush to push #193

conartist6 commented 1 year ago

@rattkin care to elaborate? I think it's a pretty crucial first step in talking about a vulnerability to figure out exactly what we are vulnerable to and how dangerous the vulnerability is.

It seems the issue being described is that sometimes the attempt to produce an error may crash. I don't think that's a real issue because we don't do any structured exception handling handling with yaml. In other words, the input for us is a yaml file which contains an exception, and the result of parsing it is an excpetion... it just isn't the one originally thrown, it'll be the exception thrown trying to generate the first exception.

The difference between the two exceptions might be consequential to certain usages, but I do not see that it has any consequence to ours.

conartist6 commented 1 year ago

That said it's still a bug that should get fixed, but I'd call it low severity. Given that I do not think we should consider fixes that require a major version bump of babel-plugin-macros.

hello-alf commented 1 year ago

Here's more details 2023-04-25 11_22_12-

conartist6 commented 1 year ago

@hello-alf that doesn't include the version the bug was introduced in. We need to know if the bug is present here.

conartist6 commented 1 year ago

The PR that created the bug in the yaml package is https://github.com/eemeli/yaml/commit/59398d2d4e3d8c33a8131a04c34c17f518d0bd70 which was part of yaml@v2.0.0-4. Thus version 1.10.2 that we depend on is definitively not vulnerable, and we can thank whoever designed the security tools for wasting our time.

conartist6 commented 1 year ago

The vuln DB is updated so the false positive should be gone.