Vulnerable Regular Expression

[cristianstaicu]

The following regular expressions used for parsing the user agent are vulnerable to ReDoS:

/(?: BePC|[ .]*fc[ \d.]+)$/i
/^ +| +$/g

The slowdown is moderate (for 50,000 characters around 4 seconds matching time). However an attacker can easily control the value of the headers he sends. I would 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.

[jdalton]

Hi again @cristianstaicu!

These regexes deal with sub sections of a UA string (so very very small strings). Your bug report is essentially "large strings cause performance problems" which could be seen as similar to "iterating large arrays cause performance problems".

If you have a patch that reworks the regexp without a {,n} limit that would rock.
PRs would be totally be welcomed.

How are you scanning projects?

[cristianstaicu]

Well, considering that an Express setup with the default configs allows user agent strings 80,000 characters long, I tend to disagree that this is just another performance problem. Anyone using your package for parsing random user agents, risks getting malicious payloads that block the main thread for anything between 5 and 10 seconds. I can send you such a payload on a private channel, if you want, and you can try it yourself. Every such request makes your webserver freeze for 5 seconds. Imagine what that means when it is being used as part of a large scale attack.

My report is the result of some manual effort for identifying ReDoS vulnerabilities in popular npm modules, but I think a tool is badly needed for automating such processes.

I don't have a patch, but I think something that limits the input string to 1,000 characters is good enough. Who needs user agents longer than this?

[jdalton]

Anyone using your package for parsing random user agents, risks getting malicious payloads that block the main thread for anything between 5 and 10 seconds.

Anyone using any utils with inputs of arbitrary length runs a performance risk. Even built-ins aren't immune (e.g. you can lock up JS with a long array spread to Math.max).

I don't have a patch, but I think something that limits the input string to 1,000 characters is good enough. Who needs user agents longer than this?

I'll accept a PR for this 👍

[jmtaillant]

Hi,
Is there a chance to have this fixed in a future release?