Braces may be vulnerable to DoS attack through snapdragon

[Gregory-Ledray]

I see this dependency chain in my package-lock.json http-proxy-middleware > braces > snapdragon > source-map-resolve > decode-uri-component

decode-uri-component has a security vulnerability CVE-2022-38900 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-38900

decode-uri-component is used by the deprecated source-map-resolve which will not fix the problem. https://github.com/lydell/source-map-resolve/issues/36

snapdragon uses source-map-resolve and hasn't addressed the issue: https://github.com/here-be/snapdragon/issues/31

Therefore I'm posting here. Is it possible to remove snapdragon as a dependency of braces?

[jonschlinkert]

Unfortunately, no, it's not possible to remove snapdragon as a dependency of that version. The library was refactored to remove snapdragon and simplify the code.

Can we patch snapdragon to use a newer version of source-map-resolve? If so I'd be happy to take a PR on snapdragon.

[Gregory-Ledray]

No @jonschlinkert source-map-resolve has no newer version and they are not taking updates.

I am looking into creating my own source-map-resolve.

source-map-resolve is used to call resolveSync which ultimately calls decode-uri-component so the dependency is "necessary" to some extent.

source-map-resolve is on v0.2.0 in decode-uri-component. It should be upgraded to v0.2.2. I reviewed the code changes between 0.2.0 and 0.2.2 and I do not see any breaking changes so I think the update in the dependency is safe. @jonschlinkert do you want me to make my own (maintained) copy of source-map-resolve whose sole change is to update decode-uri-component to v0.2.2? Do you want to do that yourself?

[jonschlinkert]

do you want me to make my own (maintained) copy of source-map-resolve whose sole change is to update decode-uri-component to v0.2.2?

This is exactly what I was going to suggest. I also was about to suggest that you publish that yourself if you wish. However, given the number of libraries that depend on this one, I'm not sure that I want to put that burden on you. also since it would be just for this library it probably makes sense to just fork that onto the micromatch org. However, I'd be more than happy to add you as a maintainer, so you can accomplish the same goal. Does that sound okay?

[Gregory-Ledray]

I would prefer it to be in the micromatch org too.

Yes, that sounds fine.

[Gregory-Ledray]

I also checked for existing forks which are maintained; the most-starred fork has only one star: https://github.com/ravikantcool2023/source-map-resolve

[jonschlinkert]

I would prefer it to be in the micromatch org too.

Okay, perfect. I sent you an invite to the org. I also forked the original source-map-resolve repo and added you as a maintainer. Thanks for helping out, I appreciate it. Let me or @doowb know if you need us to publish or review code etc.

[Gregory-Ledray]

@jonschlinkert I'm stuck. I'd like to do the following, but I can't do (1) because I can't create an Issue; the option to create Issues in the Settings in the repository is missing. I'd like to:

1. Create an Issue on micromatch/source-map-resolve to get feedback on what I've done and then tag and release a version
2. Publish to npm at source-map-resolve-2
3. Update Gregory-Ledray/snapdragon to point to source-map-resolve-2 instead of source-map-resolve
4. Create a PR in here-be/snapdragon from Gregory-Ledray/snapdragon to fix https://github.com/here-be/snapdragon/issues/31
5. Get a review on that PR
6. Merge in the PR
7. Contact github.com/lydell via an Issue on the repo to ask for them to transfer ownership of the source-map-resolve package over so we can publish to source-map-resolve instead of source-map-resolve https://docs.npmjs.com/transferring-a-package-from-a-user-account-to-another-user-account
8. Do another change and PR to switch back from source-map-resolve-2 to source-map-resolve
9. Contact / BCC other maintainers of packages using source-map-resolve and suggest to either use source-map-resolve-2 as an alternative or to deprecate their packages

Could you help with steps (1) and (2) and comment on the rest of this plan please?

I can't do (1) because of the missing Setting and I'm not sure how to do (2) properly because I have never published to npm and don't want ownership of the package on npm anyway.

@lydell your feedback would also be appreciated!

[jonschlinkert]

I enabled issues just now. I forgot that issues need to be enabled on forks. What is your NPM user name? It seems that everything else in your list should be possible.

[Gregory-Ledray]

gregoryledray

[jonschlinkert]

Added you to NPM

[lydell]

Are you using an old version of http-proxy-middleware? If so, why don’t you update it?

❯ npm i http-proxy-middleware

added 16 packages, and audited 17 packages in 1s

3 packages are looking for funding
  run `npm fund` for details

found 0 vulnerabilities

❯ npm audit
found 0 vulnerabilities

❯ npm why braces
braces@3.0.2
node_modules/braces
  braces@"^3.0.2" from micromatch@4.0.5
  node_modules/micromatch
    micromatch@"^4.0.2" from http-proxy-middleware@2.0.6
    node_modules/http-proxy-middleware
      http-proxy-middleware@"^2.0.6" from the root project

❯ npm why snapdragon
npm ERR! No dependencies found matching snapdragon

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/simon/.npm/_logs/2023-07-18T20_53_15_111Z-debug-0.log

❯ npm why source-map-resolve
npm ERR! No dependencies found matching source-map-resolve

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/simon/.npm/_logs/2023-07-18T20_53_41_324Z-debug-0.log

[Gregory-Ledray]

Lydell, I assume you're suggesting that I should solve this problem somewhere else in the dependency chain. I probably could do that, but this path seemed easier to me.

The specific path through braces which I use is from prerenderer https://github.com/chrisvfritz/prerender-spa-plugin/blob/master/package-lock.json which is recently deprecated and I want to move off of it; but for now I'm still using it.

That prerender-spa-plugin uses https://github.com/Tofandel/prerenderer/blob/dev/package-lock.json which uses "http-proxy-middleware": "^2.0.6" which uses "micromatch": "^4.0.2"

Unfortunately I do not trust the javascript dependency chains through NPM and so I prefer to mess with them as little as possible. This seemed like a good place to make a small and manageable change!

[lydell]

I would spend my time on replacing prerender-spa-plugin then. I won’t transfer ownership of source-map-resolve unless there’s an exceptionally good reason to do so. In my opinion, trying to fix these audit warnings (which will become more in the future as prerender-spa-plugin rots more) is a waste of time playing whack-a-mole for things that probably aren’t even making any of your applications vulnerable in the first place.

[Gregory-Ledray]

trying to fix these audit warnings (which will become more in the future as prerender-spa-plugin rots more) is a waste of time

I agree that it is a waste of time for me or you or Jon and that there isn't a vulnerability here. But if you consider all of the time being wasted by all of the people who read this audit warning and have to go in and do something about it (in my case, write down why I believe it is reasonable to ignore the audit warning; and then someone else will read what I wrote...) in aggregate, it'll be better for one person to fix the repo and make the warning go away than to just ignore it.

It's not fair to the maintainer, and sure it's a waste of time all around, but pointless audit warnings is where the security community is and it looks like things will be this way for the near-term future so I'm willing to spend my time to improve things for everyone.

I'll remove the bit of my plan where I ask for source-map-resolve to be transferred over to me; after source-map-resolve-2 stabilizes a bit I'll contact the other maintainers of repos which depend on source-map-resolve on npm, where possible, and ask them to switch to source-map-resolve-2. Thank you for your time; your repo was obviously a net benefit to the JS community.

[jonschlinkert]

trying to fix these audit warnings (which will become more in the future as prerender-spa-plugin rots more) is a waste of time playing whack-a-mole for things that probably aren’t even making any of your applications vulnerable in the first place

I completely agree with this usually, and it's why this hasn't been addressed to this point. However, I won't stop someone else from working on it. Also, we've received dozens or more issues related to this specific bug in source-map-resolve, so it would be nice to get it resolved.

In general, I agree with your point though. We get issues constantly for things that aren't even remotely vulnerabilities, but people receive bounties for it and it's a losing battle to try to explain this every time.

I won’t transfer ownership of source-map-resolve unless there’s an exceptionally good reason to do so.

I didn't ask, I understand this perspective.

[lydell]

You misunderstood me. I wasn’t advocating for ignoring the audits – I like keeping them to a minimum myself. I was advocating for getting rid of your audits the right way – which is to “prune your tree” as close to the trunk as possible. This thread feels like fixing a leaf while the whole branch is ill. The problem is already solved – the latest micromatch and braces already are nice and up-to-date and audit free, the issue is that people are using deprecated things elsewhere that pull in old micromatch or braces. They should fix those deprecations and get a healthier npm dependency tree overall.

[Gregory-Ledray]

I see what you mean in https://github.com/micromatch/braces/issues/34#issuecomment-1640999572 now. https://github.com/Tofandel/prerenderer/blob/dev/package-lock.json which I previously referenced is actually fixed.

The issue is that the up-to-date version of https://github.com/chrisvfritz/prerender-spa-plugin/blob/master/package-lock.json is on 0.7.2 of @prerenderer/prerenderer which is a very old version. I can't find the source Github tag but can see the code on npm referencing the dependency chain I see in my package-lock.json. That package was deprecated so won't receive updates. Its successor was also deprecated, but that package's successor https://github.com/Tofandel/prerenderer is actively maintained and has fixed the problem. So I could just switch to that package and make some code changes.

Hm.

So my options are:

• Ignore all of this and switch to https://github.com/Tofandel/prerenderer which would fix the problem
• Create a new source-map-resolve and publish and then point the old version of snapdragon there

Doing the first one would make more sense for me personally.

My only remaining reason for doing the second one is altruism to prevent other people from seeing this issue. But if my experience is typical then the right way for them to address the situation would also be to move to a maintained package or version. So... ignoring this is the best option.

[Gregory-Ledray]

@jonschlinkert sorry about asking you to do work; it seems you have a lot of packages and better things to do with your time. If you wish, you could delete https://github.com/micromatch/source-map-resolve (although I believe it is ready-to-go).

I don't see any orgs on my npm; if you added me there I don't think it worked.

[lydell]

My only remaining reason for doing the second one is altruism to prevent other people from seeing this issue. But if my experience is typical then the right way for them to address the situation would also be to move to a maintained package or version.

That’s very kind and open source spirit! It makes me happy reading something like that. But in this case, I don’t think it’s worth it, because even if you get rid of the audit warnings in this case (until the next ones pop up), people still gain more from upgrading their stuff instead. micromatch 3 to 4 (for example) was a huge improvement in my opinion.

[jonschlinkert]

the issue is that people are using deprecated things elsewhere that pull in old micromatch or braces

If we wait for "people" to do what they should do, nothing will ever get done.