Vulnerable Regular Expression

[cristianstaicu]

The following regular expression used to parse dates specified as strings is vulnerable to ReDoS:

/[0-9]*['a-z\u00A0-\u05FF\u0700-\uD7FF\uF900-\uFDCF\uFDF0-\uFFEF]+|[\u0600-\u06FF\/]+(\s*?[\u0600-\u06FF]+){1,2}/i

The slowdown is moderately low: for 50.000 characters around 2 seconds matching time. However, I would still suggest one of the following:

• remove the regex,
• anchor the regex,
• limit the number of characters that can be matched by the repetition,
• limit the input size.

If needed, I can provide an actual example showing the slowdown.

[mattjohnsonpint]

For clarification, this is in the matchWord regex, in /src/lib/parse/regex.js here.

Here is a railroad diagram of the regular expression. From this we can see that the grouping with repetition is related to parsing Arabic characters. It would be helpful if someone who understands both regular expressions and Arabic language could take a crack at this.

An overview of ReDoS is also helpful.

[hamiltondanielb]

Ill take this is no one has already started

[icambron]

@hamiltondanielb all yours :)

[drag0s]

@cristianstaicu
If needed, I can provide an actual example showing the slowdown.

I'd like to see a example if it's possible :)

[cristianstaicu]

@drag0s sent it on your private email.

[mattgrande]

FYI, this was added to NSP (see here), so this is probably going to start breaking people's builds soon.

@hamiltondanielb - Did you get anywhere looking into this?

[jonsharratt]

Just had our build break 👯

[dskrvk]

It's sad that the issue had to become public before it was fixed. Issue opened on Sep 8, NSP advisory published today. @cristianstaicu perhaps you should've reminded the maintainers about the disclosure deadline to give this some momentum.

[rupesh1]

@mattgrande there is a meta-ness to this: turns out the version of nsp we were pinned to (2.8.1) depends on moment (via joi) so it was reporting a vulnerability on its own dependency:

Upgrading to nsp 3.1.0, resolved this because the dependency is no longer there - so beware of that if you don't directly depend on moment.

[bit7]

Is there a fix for this yet?

[JoanYin]

Please advise any fix available?

[fluxsauce]

No fix has been published yet.

Please, if you're interested in getting updates from the maintainers, subscribe to notifications for updates to this issue by clicking the "Subscribe" in the right-hand column.

[westy92]

To add an nsp exception for this, add a .nsprc file:

{
  "exceptions": [
     "https://nodesecurity.io/advisories/532"
  ]
}

[jacob-go]

Thanks @westy92 ! Saved my build.

[Dexterslab]

Hi @westy92 and @jacob-go . I have the following code.
var tasksMethods = require('gulpfile-ninecms');
gulp.task('nsp', tasksMethods.nsp);
Its not picking up the .nsprc file exception. It keeps giving me the vulnerability error for moment.
I added the file on root of the project. Is there anything I am missing to do?

[westy92]

@Dexterslab we're using gulp-nsp, which works fine when the .nsprc is in the project directory (same level as package.json). Maybe try using gulp-nsp directly?

[nicosommi]

@cristianstaicu @mattgrande is this happening in Luxon as well?

[davidnormo]

We'd appreciate it the fix for this can be expedited, now that it has been logged in nsp it is failing our builds.