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 #82

Open
cristianstaicu opened this issue Sep 6, 2017 · 8 comments
Open

Vulnerable Regular Expression #82

cristianstaicu opened this issue Sep 6, 2017 · 8 comments

Comments

@cristianstaicu
Copy link

The following regular expression used in parsing the input string is vulnerable to ReDoS:

/^\s+|\s+$/g

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.

@evilpacket
Copy link

As this issue is public, we've issued an advisory here https://nodesecurity.io/advisories/537 as well as requested help from the public to submit a PR / help patch this issue.

@wesleytodd
Copy link

If it helps anyone, here are some references for similar issues which have been fixed in different ways:

jshttp/forwarded@d469116
jshttp/fresh@21a0f0c
broofa/mime@1df903f
debug-js/debug@f53962e

I don't have the time to work on this right now, but we have dealing with these issues reported by @cristianstaicu for the past couple of weeks, so it was easy for me to pull up these references as to how others have addressed the ReDOS issues.

@vvanmol
Copy link

vvanmol commented Jan 24, 2018

Hello,

One of the NPM package we use had been blocked by our Nexus IQ Server because it has a dependency to slug which is blocked due to this vulnerability...

I would like to fix this and I have checked the different examples of solution provided by @wesleytodd but in this case the RegEx is used to trim a String...

According to you people would the use of the String.prototype.trim function be a valid solution for this case ? I'm no Security expert but I would definitely like to help...

vvanmol pushed a commit to vvanmol/node-slug that referenced this issue Jan 24, 2018
@cutesquirrel
Copy link

👍

@Trott
Copy link

Trott commented Oct 25, 2018

0.9.2 has been published with this issue fixed. https://www.npmjs.com/package/slug/v/0.9.2

@cristianstaicu Can you please close this issue?

@rciccone91
Copy link

Hi guys, just one quick question, is this issue really fixed? Our Nexus IQ Server is still detecting this vulnerability. Maybe it's because this issues has not been closed yet?
Thanks!

@Trott
Copy link

Trott commented Dec 10, 2018

is this issue really fixed?

Yes.

Our Nexus IQ Server is still detecting this vulnerability. Maybe it's because this issues has not been closed yet?

Alas, I have publishing rights on the npm module but no write privileges on this repository. So I can publish a fix (which I have) but I can't close this issue.

Do make sure you are using 0.9.2. Previous versions do not have the fix.

@vvanmol
Copy link

vvanmol commented Dec 11, 2018

Hello,

Sorry @Trott for the late reply. And thank you for taking over this package.

  • Vulnerability had been fixed and confirmed by security tools like Snyk (0.9.2 and 0.9.3).
  • npmjs.org official package page now refers to @Trott Github repository.

Thanks !!

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

7 participants