node-slug icon indicating copy to clipboard operation
node-slug copied to clipboard
verified publisher icon dodo

Vulnerable Regular Expression

Open cristianstaicu opened this issue 8 years ago • 8 comments

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.

cristianstaicu avatar Sep 06 '17 14:09 cristianstaicu

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.

evilpacket avatar Sep 28 '17 00:09 evilpacket

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

https://github.com/jshttp/forwarded/commit/d469116eda4931fbe1c0ccb29497b35930bfa328 https://github.com/jshttp/fresh/commit/21a0f0c2a5f447e0d40bc16be0c23fa98a7b46ec https://github.com/broofa/node-mime/commit/1df903fdeb9ae7eaa048795b8d580ce2c98f40b0 https://github.com/visionmedia/debug/commit/f53962e944a87e6ca9bb622a2a12dffc22a9bb5a

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.

wesleytodd avatar Oct 11 '17 19:10 wesleytodd

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 avatar Jan 24 '18 14:01 vvanmol

👍

cutesquirrel avatar Jun 13 '18 08:06 cutesquirrel

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?

Trott avatar Oct 25 '18 05:10 Trott

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!

rciccone91 avatar Dec 10 '18 18:12 rciccone91

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.

Trott avatar Dec 10 '18 19:12 Trott

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

vvanmol avatar Dec 11 '18 08:12 vvanmol