search

TooTallNate / plist.js

Mac OS X Plist parser/builder for Node.js and browsers
MIT License
571 stars 123 forks source link

Update xmldom to 0.7.x #112

Closed dylmye closed 3 years ago

dylmye commented 3 years ago

The name of the package changed due to the owner abandoning it. More info at xmldom/xmldom#271.

This fixes #110, fixes #111.

dylmye commented 3 years ago

Didn't notice the whitespace changes (thanks Prettier) - I'll remove this in the morning if nobody else gets to it first

SudiptaAtWork commented 3 years ago

Why title is saying update xmldom to 0.7.1, when it is 0.7.0. It is creating confusion as it seems it is updating to some beta version

dylmye commented 3 years ago

Apologies @SudiptaAtWork, this has been corrected

dylmye commented 3 years ago

/cc @mreinstein

SudiptaAtWork commented 3 years ago

Any update when will this PR be closed and a new version released for plist?

brodycj commented 3 years ago

xmldom (@xmldom/xmldom) is now up to 0.7.2 ... is there anything we can do to help get this updated?

Apache Cordova has been using this wonderful package as well.

mreinstein commented 3 years ago

Where to begin with this whole clusterfuck....

dylmye commented 3 years ago

Hey Mike, thanks for checking in on this. Sorry for the bother! This is going to be a bit long, sorry. The TL;DR is:

Where to begin with this whole clusterfuck....

Here's my understanding of the timeline:

  1. xmldom was created by Dev A and last updated in 2017
  2. After a few years of being abandoned, the package was picked up by a developer (Dev B), under the newly-created xmldom org.
  3. Dev B adds a number of other interested contributors to the org and contacts npm to gain control of the xmldom package under their abandoned package/dispute policy (see here) - publishes versions 0.3.0 - 0.6.0 (0.6.0 pushed out 17 April 2021.)
  4. A vuln advisory is published on the 27th of July 2021.
  5. At some point between April and July, npm changes their policy on abandoned packages and relinquishes control from Dev B and their contributors without permission from Dev A.
  6. NPM/GitHub refuses to give control and Dev A is radio silent, so Dev B & co publish the same package (with security fix) under the @xmldom npm org.

how did we get into a situation where noone with github access to xmldom has npm access?

✨ magic ✨ aka NPM's current dispute policy, updated 16 July:

We are not currently accepting dispute requests to "adopt an abandoned package" or "Report Squatting" as we re-evaluate and update the overall dispute process.

what happened to the original maintainers?

Dev A hasn't got any public activity on their GitHub or NPM. The other contributor has published stuff on GitHub since Dev B & co reached out to Dev A. I don't know if they reached out to the other contributor.

I fully understand how busy you are - hopefully this PR helps and we can get more community support. Also would love input from Nate if possible.

Has anyone evaluated if this security issue actually affects plist.js

XMLDom's DOMParser is used in the parse() method. The affected method from their library is serializeToString() which we don't use. However switching to this package a) stops the warnings, and b) makes it easier to mitigate serious issues in the future that might affect the project.

Hope this helps!

brodycj commented 3 years ago

I have just added some quick background to https://github.com/xmldom/xmldom/issues/271, with quick post-mortem in this comment: https://github.com/xmldom/xmldom/issues/271#issuecomment-902249697

I would highly recommend asking any questions for clarification in https://github.com/xmldom/xmldom/issues/271.

mreinstein commented 3 years ago

@dylmye thanks for taking the time to write up more details. Here are my current thoughts on potential solutions:

  1. copy the used parts of xmldom@0.6 into plist.js. It sounds like this is not really a critical severity for plist.js as we're not using the vulnerable parts of the xmldom package. This will eliminate the noise about vulnerabilities without introducing a new package to plist.
  2. patch release (plist@3.0.4)
  3. refactor plist to use another library (currently leaning towards htmlparser2)
  4. resolve some other open issues (ideally lumping in other potentially breaking changes like parser fixes)
  5. remove support for node < 12.17
  6. pure esm module (drop legacy commonjs)
  7. major release (plist@4 )

Open for suggestions/thoughts/ideas on this.

SudiptaAtWork commented 3 years ago

Any ETA on when this PR will be complete? Our app is showing security vulnerability because of xmldom 0.6.0, and we do not use xmldom directly it is plist which is injecting xmldom in our package.lock file. Without plist fixing this we can not get through this vulnerability checkpoint. Please any update sooner will be helpful. At least if a temporary solution of using xmldom 0.7.2 can be checked in and a version can be created and you take other issues in a separate version, then it will be helpful for us who has indirect dependency on xmldom for plist.

mreinstein commented 3 years ago

resolved via https://github.com/TooTallNate/plist.js/commit/fa8e184631d3b809da1a9e3cfcf6407919871d1b (steps 1-2 in my previous comment.)

Steps 3-7 moved to #113

karfau commented 3 years ago

The codebase of (xmldom which is the same as) @xmldom/xmldom luckily is under active development again and we are fixing various bugs in every patch and minor release (as of writing 0.7.4 has been released and 0.8.0 is being prepared). If there are issues in this library because of the parsing done in xmldom, chances are high that we would love to fix them.

Of course you have the right to inline the sources into you project, I hope it also makes it easier for you to manage/own the contained bugs. As you can see from the issues that are already filed, there are also some that impact xml parsing: https://github.com/xmldom/xmldom/issues?q=is%3Aissue+is%3Aopen+label%3Abug%2Cspec%3AXML+ , https://github.com/xmldom/xmldom/issues/69 being one that might be relevant for you which was fixed in 0.7.0

PS: I did see your plan to switch to another library, curious which one you will pick and be more confident about the level of maturity regarding parsing. I'm not claiming we are far ahead, just didn't look into alternatives since I joined the contriutors/maintainers.