Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ReDoS in unmaintained path-parse dependency #240

Closed
TayHobbs opened this issue Apr 13, 2021 · 13 comments
Closed

ReDoS in unmaintained path-parse dependency #240

TayHobbs opened this issue Apr 13, 2021 · 13 comments

Comments

@TayHobbs
Copy link

path-parse has a security vulnerability that has been reported but not fixed in two years (jbgutierrez/path-parse#8). Additionally, it still has not enabled 2fa. Is it possible for this project to remove path-parse as a dependency or move to a new library?

Thank you!

@ljharb
Copy link
Member

ljharb commented Apr 13, 2021

How can you possibly know if it’s enabled 2FA?

For the record, i don’t consider this a vulnerability in path-parse or in resolve - if you are using resolve with unsanitized user-created paths, hen you have the vulnerability.

Do you have an alternative library to suggest?

@TayHobbs
Copy link
Author

Apologies, meant to link this issue in my initial comment: jbgutierrez/path-parse#5 is what I'm using as my basis for not having 2FA.

Potentially parse-path which looks well-maintained and the documentation says it works with local paths or native node.js path.parse.

Resolve is a dependency of a dependency in our projects and path-parse showed up as a moderate security vulnerability in our report. We are looking to reduce our security vulnerability report to as low as possible. As I did not see any issues referencing the unmaintained dependency in this project, I thought I would make an issue to alert the maintainers.

Thank you for your time

@ljharb
Copy link
Member

ljharb commented Apr 13, 2021

That has a lot of dependencies - and two of them are query string parsers. I'd prefer to find something simpler. We also need to retain node compatibility < v0.6.

In your report, is there a link to a public CVE or vulnerability report? or is there just that one issue?

Either way, you could use (once they exist) npm overrides to replace path-parse with require('path').parse if you know you have a newer node version.

@TayHobbs
Copy link
Author

Our report just points to that issue. Thank you for the guidance, we'll work with that and create an exception for path-parse if resolve is not concerned with it.

@ljharb
Copy link
Member

ljharb commented Apr 13, 2021

I don't think it's a legitimate security concern, no - but if there's an easy way to avoid false positive warnings like this, I'm happy to accept such a change.

@TayHobbs
Copy link
Author

I think the easiest fix is for path-parse to address or close the issue so it doesn't show up in reports, but that is not something resolve can fix. At this point, I think we can close this issue based on our discussion.

@ljharb ljharb closed this as completed Apr 13, 2021
@alasdairhurst
Copy link

The public CVE for this issue is https://nvd.nist.gov/vuln/detail/CVE-2021-23343

@modestfake
Copy link

We also need to retain node compatibility < v0.6.

@ljharb just curious, why do you want to support < v0.6? Current LTS versions of Node.js are 12 and 14.

@ljharb
Copy link
Member

ljharb commented May 6, 2021

For one, because we already do, and it would be semver-major to drop it.

Separately, LTS status is irrelevant; just because node won’t support it doesn’t mean package authors shouldn’t.

@sfowl

This comment has been minimized.

@ljharb

This comment has been minimized.

@ljharb ljharb reopened this May 12, 2021
@ljharb

This comment has been minimized.

@ljharb
Copy link
Member

ljharb commented May 25, 2021

v1.0.7 of path-parse should address the vulnerability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

5 participants