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

Vulnerable Regular Expression #4163

Closed
cristianstaicu opened this issue Sep 8, 2017 · 24 comments
Closed

Vulnerable Regular Expression #4163

cristianstaicu opened this issue Sep 8, 2017 · 24 comments

Comments

@cristianstaicu
Copy link

@cristianstaicu cristianstaicu commented Sep 8, 2017

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
Copy link
Member

@mattjohnsonpint mattjohnsonpint commented Oct 19, 2017

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
Copy link

@hamiltondanielb hamiltondanielb commented Oct 20, 2017

Ill take this is no one has already started

@icambron
Copy link
Member

@icambron icambron commented Oct 20, 2017

@hamiltondanielb all yours :)

@drag0s
Copy link

@drag0s drag0s commented Oct 24, 2017

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

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

@cristianstaicu
Copy link
Author

@cristianstaicu cristianstaicu commented Oct 24, 2017

@drag0s sent it on your private email.

@mattgrande
Copy link
Contributor

@mattgrande mattgrande commented Nov 27, 2017

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
Copy link

@jonsharratt jonsharratt commented Nov 27, 2017

Just had our build break 👯

@dskrvk
Copy link

@dskrvk dskrvk commented Nov 27, 2017

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
Copy link

@rupesh1 rupesh1 commented Nov 27, 2017

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

1__bash

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
Copy link

@bit7 bit7 commented Nov 27, 2017

Is there a fix for this yet?

@JoanYin
Copy link

@JoanYin JoanYin commented Nov 27, 2017

Please advise any fix available?

@fluxsauce
Copy link

@fluxsauce fluxsauce commented Nov 27, 2017

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
Copy link

@westy92 westy92 commented Nov 27, 2017

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

{
  "exceptions": [
     "https://nodesecurity.io/advisories/532"
  ]
}
@jacob-go
Copy link

@jacob-go jacob-go commented Nov 27, 2017

Thanks @westy92 ! Saved my build.

@Dexterslab
Copy link

@Dexterslab Dexterslab commented Nov 27, 2017

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
Copy link

@westy92 westy92 commented Nov 27, 2017

@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
Copy link

@nicosommi nicosommi commented Nov 27, 2017

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

@davidnormo
Copy link

@davidnormo davidnormo commented Nov 28, 2017

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

kmerz added a commit to Graylog2/graylog2-server that referenced this issue Mar 7, 2018
The javascript package moment.js had a vulnerability regarding
regular expresions.

moment/moment#4163

This change updates moment.js to a fixed version.
kmerz added a commit to Graylog2/graylog2-server that referenced this issue Mar 7, 2018
The javascript package moment.js had a vulnerability regarding
regular expresions.

moment/moment#4163
https://github.com/moment/moment/blob/2.21.0/CHANGELOG.md

This change updates moment.js to a fixed version.
bernd added a commit to Graylog2/graylog2-server that referenced this issue Mar 8, 2018
The javascript package moment.js had a vulnerability regarding
regular expresions.

moment/moment#4163
https://github.com/moment/moment/blob/2.21.0/CHANGELOG.md

This change updates moment.js to a fixed version.
machristie added a commit to apache/airavata-django-portal that referenced this issue Mar 9, 2018
2.19.1 has regular expression denial of service (ReDoS) vulnerability.
See moment/moment#4163 for details.
octoquad pushed a commit to octoquad/RigMon that referenced this issue Apr 3, 2018
* Bump up version for moment from 2.19.1 to fix ReDOS vulnerability. See
moment/moment#4163
* Adding package-lock.json
trentm pushed a commit to trentm/node-bunyan that referenced this issue Jun 28, 2020
CVE-2017-18214
https://nodesecurity.io/advisories/532

See the upstream issue resolved by the update:
moment/moment#4163
trentm added a commit to trentm/node-bunyan that referenced this issue Jun 29, 2020
justbill2020 added a commit to justbill2020/moment-timer that referenced this issue Feb 8, 2021
- corrects advisories 55 & 532

moment/moment#4163
moment/moment#4326

Update package.json and readme with this repo.
justbill2020 added a commit to justbill2020/moment-timer that referenced this issue Feb 19, 2021
- corrects security advisories 55 & 532

moment/moment#4163
moment/moment#4326

Update package.json and readme.
justbill2020 added a commit to justbill2020/moment-timer that referenced this issue Feb 19, 2021
- updated moment.js version in vendor folder
- added r.js (require.js)
- created nodejs example

Minor version bump, Security fix

- corrects security advisories 55 & 532

moment/moment#4163
moment/moment#4326

Update package.json and readme.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet