NaturalIntelligence / fast-xml-parser

Validate XML, Parse XML and Build XML rapidly without C/C++ based libraries and no callback.
https://naturalintelligence.github.io/fast-xml-parser/
MIT License
2.45k stars 296 forks source link

Vulnerability open for a few days: GHSA-gpv5-7x3g-ghjv #591

Closed alfaproject closed 1 year ago

alfaproject commented 1 year ago

All the details here: https://github.com/advisories/GHSA-gpv5-7x3g-ghjv

This was opened a few days ago, but it looks like it was already patched? Or is it a false positive?

Either way, a release can be done or the vulnerability discarded, but as it stands our pipelines are stopped right now unless we ignore this but I'd prefer not to ignore without getting feedback here.

github-actions[bot] commented 1 year ago

We're glad you find this project helpful. We'll try to address this issue ASAP. You can vist https://solothought.com to know recent features. Don't forget to star this repo.

jozefsukovsky commented 1 year ago

I would like to add, that this report breaks auditing tools, e.g. pnpm audit (which can not omit/skip GHSA reports). And the report itself is a bit confusing, as it is not clear if it really is a security issue or just a suggestion. Hence, the consequences of this GHSA are quite unbalanced.

WikiRik commented 1 year ago

This seems to have been fixed in https://github.com/NaturalIntelligence/fast-xml-parser/commit/9a880b887916855c3a510869fd1ee268d7fe58b1 but a new release has not been published for this yet

Afellman commented 1 year ago

Any update on this?

felixsonyusuftosin commented 1 year ago

Will this be updated anytime soon?

martin-walsh commented 1 year ago

@amitguptagwl your published advisory on your most recent security fix means fix is hard to adopt https://github.com/advisories/GHSA-gpv5-7x3g-ghjv

gijspon commented 1 year ago

Need an update for this!

aaleksandrov commented 1 year ago

Would be awesome to release this - aws-sdk-v3 depends on this library and brings this vulnerability

eclousersans commented 1 year ago

This seems to have been fixed in 9a880b8 but a new release has not been published for this yet

Would like to see a quick release for this, as the issue is currently blocking many of our CI pipelines.

Zajozor commented 1 year ago

If you want to "hotfix"/"force" your yarn audit's to pass in the meantime, here's a tip:

Lockfile part of the library that depends on fast-xml-parser

---    fast-xml-parser "4.2.4"
+++    fast-xml-parser "https://github.com/YourGreatFork/fast-xml-parser#v4.2.5"

Lockfile part where fast-xml-parser is declared:

--- fast-xml-parser@4.2.4:
---  version "4.2.4"
+++ fast-xml-parser "https://github.com/YourGreatFork/fast-xml-parser#v4.2.5":
+++   version "4.2.5"
+++   resolved "https://github.com/YourGreatFork/fast-xml-parser#cc73065e1469147a0104dc122b0cdf6724354446"

(note that the commitID is the latest commit ID in the main branch of this repo - check it yourself to be sure!)

It's obviously not pretty, and manual lockfile edits are not a good idea in general, but it may be an acceptable temporary hotfix for your case. It should survive subsequent npm/yarn install's.

Looking forward to an actual release including the fix on the upstream package :) !

fernandopioli commented 1 year ago

Hi @amitguptagwl. Could you please help us on that? thanks

amitguptagwl commented 1 year ago

I'm little confused here. Someone has raised a PR to update GitHub Advisory Database which is not in my control. But there is no open security issue that I know. What should I do to solve the issue? So you guys should not see any error in your build pipeline

amitguptagwl commented 1 year ago

Now I've updated all the advisories with fixed version. Please check if this solve your issue.

aaleksandrov commented 1 year ago

@amitguptagwl , as @WikiRik pointed here a fix was already merged so it's a matter of releasing a new version as far as I understand

amitguptagwl commented 1 year ago

@aaleksandrov I have that too few mins ago. So I hope everything is shorted now.

aaleksandrov commented 1 year ago

Thanks, now aws-sdk-v3 needs to release an update because they have the fast-xml-parser version pinned as 4.2.4

7085 commented 1 year ago

This should not have been a security advisory in the first place but a regular issue, because the actual vulnerability was already fixed in 4.2.4. Now there are two, you should revoke those. Only publish an advisory when there is an actual vulnerability, people misuse the button "Report a vulnerability" often unfortunately.

eclousersans commented 1 year ago

Our issue is now resolved. Thanks!

amitguptagwl commented 1 year ago

I'm closing this issue. But if anything left then please reopen or comment.