pimterry / loglevel

:ledger: Minimal lightweight logging for JavaScript, adding reliable log level methods to wrap any available console.log methods
MIT License
2.62k stars 157 forks source link

Security Vulnerability: Prototype Pollution - CVE-2024-38998 #199

Closed stpru24 closed 2 months ago

stpru24 commented 2 months ago

Are there any chances of getting a security patch to address this issue? https://nvd.nist.gov/vuln/detail/CVE-2024-38999

Also a high severity vulnerability published by github: https://github.com/advisories/GHSA-x3m3-4wpv-5vgc

We are getting this on security scans due to grunt-template-jasmine-requirejs

pimterry commented 2 months ago

We are getting this on security scans due to grunt-template-jasmine-requirejs

This sounds like a bug in your security scanner, or like you're vendoring this package unnecessarily for some reason. This package doesn't have any runtime dependencies! Zero. The dependency with this security warning is a development dependency that you shouldn't be installing at all, unless you're doing development work on this package. Does that make sense?

PRs are welcome to update that dependency if you like, but this repo is very old now and the package is effectively 'done' and now in maintainence mode, so it would be best to do the minimal possible change to support that and avoid major changes where possible.

esantonja commented 2 months ago

Hi @pimterry.

I'm checking this issue and looks like the problem is related about how the npm package is generated.

The npm package published contains the grunt-template-jasmine-requirejs files that are part of this code base, however those files are not needed when we use this package as a dependency.

I'm guessing the package is published just running npm publish. This is encapsulating all the code in the repo as this is the default behavior.

Would be feasible to update the release process so we copy only the required files (or at least not add testing code)?

pimterry commented 2 months ago

Ah, that's interesting! Good catch, yes, we don't need to deploy that. It's not an actual security problem regardless, since this is never used at runtime, but it is a bit pointless and I can see how that could trigger alerts.

I've now dropped that from the package files, and re-released as 1.9.2. Does that work for you?

esantonja commented 2 months ago

Hi @pimterry!

It worked like a charm! Our vulnerabilities check marks the new version as clean.

Thank you very much!

stpru24 commented 2 months ago

Thanks for resolving this.