tyrasd / osmtogeojson

convert osm to geojson
http://tyrasd.github.io/osmtogeojson/
MIT License
683 stars 112 forks source link

Vulnerability in @xmldom/xmldom 0.8.3 #139

Open kachkaev opened 1 year ago

kachkaev commented 1 year ago

πŸ‘‹ @tyrasd! It’d be great to see this dependency upgraded, currently seeing:

yarn npm audit
└─ @xmldom/xmldom: 0.8.3
   β”œβ”€ Issue: xmldom allows multiple root nodes in a DOM
   β”œβ”€ URL: https://github.com/advisories/GHSA-crh6-fp67-6883
   β”œβ”€ Severity: critical
   β”œβ”€ Vulnerable Versions: >=0.8.0 <0.8.4
   β”œβ”€ Patched Versions: >=0.8.4
   β”œβ”€ Via: @xmldom/xmldom
   └─ Recommendation: Upgrade to version 0.8.4 or later
yarn why @xmldom/xmldom
β”œβ”€ osmtogeojson@npm:3.0.0-beta.5
β”‚  └─ @xmldom/xmldom@npm:0.8.3 (via npm:0.8.3)
β”‚
└─ root-workspace-0b6124@workspace:.
   └─ @xmldom/xmldom@npm:0.8.6 (via npm:^0.8.6)
yarn why osmtogeojson
└─ root-workspace-0b6124@workspace:.
   └─ osmtogeojson@npm:3.0.0-beta.5 (via npm:^3.0.0-beta.5)

This issue is fixable by https://github.com/tyrasd/osmtogeojson/pull/138, but you can bump min version further (0.8.4 β†’ 0.8.6 at the time of writing).

yuiseki commented 1 year ago

Hi, Thanks for developing such great software, @tyrasd ! And thanks for suggesting the Issue related xmldom then create Pull request fix that, @kachkaev !

Please allow me to share with you a link to the GitHub Advisory on xmldom: https://github.com/advisories/GHSA-crh6-fp67-6883

GitHub Advisory marked severity of this vulnerability is Critical, Therefore, we are very sorry for the trouble, but please do this dependency update πŸ™

johnlettman commented 1 year ago

Hello everyone! I want to reach out and mention that I appreciate the existence of this project. Thank you @tyrasd! It's helped me write a tiny webpack loader for Overpass API queries.

As mentioned in previous comments, the @xmldom/xmldom dependency appears to be pinned to the vulnerable version. In PR #138 the comment referencing a minimum version rather than a version pin could alleviate the vulnerability pretty succinctly: https://github.com/tyrasd/osmtogeojson/pull/138#discussion_r1084546006

Though my project never uses the XML portion of osmtogeojson, I imagine others may have the alert, with their projects remaining vulnerable. Please consider merging; I would happily lend a hand in testing for any regressions.

Cheers! :)

johnlettman commented 1 year ago

Hello friends,

I would like to share the temporary workaround I've used in my package here: https://github.com/johnlettman/overpassql-loader/issues/6 https://github.com/johnlettman/overpassql-loader/commit/ead1f13970d471eafd4e5f52645840d62e7420d9

It involves using the Snyk PR to resolve the CVE: https://github.com/tyrasd/osmtogeojson/pull/138

Essentially, you can change your dependency from NPM to the GitHub repository with the branch Snyk is making the PR from (https://github.com/tyrasd/osmtogeojson/tree/snyk-fix-65371a4c4920389f7e5127c141088511)

Just run:

yarn add "https://github.com/tyrasd/osmtogeojson#snyk-fix-65371a4c4920389f7e5127c141088511"
kachkaev commented 9 months ago

@tyrasd πŸ™