Xpra-org / xpra-html5

HTML5 client for Xpra
Mozilla Public License 2.0
209 stars 55 forks source link

"braces" v3.0.2 / "micromatch" v4.0.5 vulnerabilities #306

Closed sbradnick closed 4 months ago

sbradnick commented 4 months ago

Getting a report of: "A vulnerable version (3.0.2) of the braces package is embedded in" and I do see this:

$ grep -ir braces .
...
./xpra-html5/package-lock.json:    "node_modules/braces": {
./xpra-html5/package-lock.json:      "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz",
./xpra-html5/package-lock.json:        "braces": "^3.0.2",
./xpra-html5/package-lock.json:    "braces": {
./xpra-html5/package-lock.json:      "resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz",
./xpra-html5/package-lock.json:        "braces": "^3.0.2",

Some references here: https://github.com/micromatch/braces/issues/35 but there hasn't been a commit to that repo in 5 years.

EDIT: I do see this too: https://github.com/micromatch/braces/pull/37 - hopefully that moves forward.

totaam commented 4 months ago

I'm not really bothered about build time stuff. Also happy to remove the lot.

totaam commented 4 months ago

TBH, this came from a drive-by contribution which is causing me pain to this day. Can we just get rid of all this nonsense? How?

sbradnick commented 4 months ago

TBH, this came from a drive-by contribution which is causing me pain to this day. Can we just get rid of all this nonsense? How?

The whole of npm & package-lock.json was someone else's additon? I'm not much of a node user (unless it makes an appearance in something else I'm trying to use and I can just let it 'do it's thing'), so I don't know if those offending additions can simply be removed w/o causing some type of node dependency problem. "micromatch" requires "braces" / "micromatch" is required by "fast-glob"/"lint-staged", that type of thing ...

Another CVE was added too: https://github.com/micromatch/micromatch/issues/243 (the comments in that one, wow ...)

$ grep -ir micromatch .
./package-lock.json:        "micromatch": "^4.0.4"
./package-lock.json:        "micromatch": "^4.0.4",
./package-lock.json:    "node_modules/micromatch": {
./package-lock.json:      "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz",
./package-lock.json:        "micromatch": "^4.0.4"
./package-lock.json:        "micromatch": "^4.0.4",
./package-lock.json:    "micromatch": {
./package-lock.json:      "resolved": "https://registry.npmjs.org/micromatch/-/micromatch-4.0.5.tgz",
totaam commented 4 months ago

It started relatively small: 41d3c2dc2f2606c5ebfb5f9ff9086bad385c9534 And now we have a 6617 lines monster. When I say remove, I'm thinking of just removing everything node related. It doesn't help me get things done in any shape or form, and just like this ticket, it causes pain.

sbradnick commented 4 months ago

When I say remove, I'm thinking of just removing everything node related.

Makes sense to me, I couldn't even begin to tell you what any of those packages are adding and if xpra-html5 functions just fine w/o them (and their extra complexity), it seems like a good thing to remove. And if there's any type of "braces" and "micromatch" CVE resolution, maybe they could be added back (I'm certainly not advocating that, I didn't even know they were a part of xpra-html5 until I got a bug report assigned to me 😜) if there is some type of value-add [to someone].

totaam commented 4 months ago

I couldn't even begin to tell you what any of those packages are adding and if xpra-html5 functions just fine w/o them

These are just commit or push hooks. They add nothing to xpra, they are not bundled with an installation, not even in the source releases.

So, the risk of harm from these two CVEs is pretty much zero. I can control-C if somehow my build takes too long.

totaam commented 4 months ago

Also helps with #277 by not having to track down all of this crap

sbradnick commented 4 months ago

Thanks to your explanation of how "braces" and "micromatch" are (not really) integrated to xpra-html5, the bugs (on my end) have been closed, so I'll close this. I really appreciate you conversing with me about it.

paulmillr commented 4 months ago

There is NO vulnerability: https://github.com/micromatch/braces/pull/37#issuecomment-2121649614

totaam commented 4 months ago

Nonetheless, there may well be other vulnerabilities in this monstrous node lock file, and none of them matter because they're not even being used. So I still want to get rid of this mess.