ReDoS (Regular Expression Denial of Service, S4784) detection could be smarter

Rules and Languages Report False-positive / False-negative...
1

Versions used

  • SonarQube Version 8.3 (build 34182)
  • SonarJS 6.2.1 (build 12157)

The problem

After a recent upgrade on the Sonar server I use, this rule got added and flagged a bunch of regexes in my codebase. A large majority of those cases can be trivially determined as not vulnerable, if I understand the source document correctly.

The attack requires a group which has a repeat operator applied to it. So, /(a+)+/ is vulnerable but /(a+)b+/ is not. If I look at my code and all the groups have no repeat operator, that’s a false positive.

The solution

I’m not a regex black-belt, but as far as I know the only repeat operators are *, +, and {n} (for certain values of n but I won’t even get into that). This means that vulnerable code must contain a close-paren ) followed by one of these characters.

The help text for the current implementation of this rule says

This rule flags any execution of a hardcoded regular expression which has at least 3 characters and at least two instances of any of the following characters: *+{ .

I would propose adding a check that the regex contains one of these 3 substrings: )+, )*, or ){.

2

Hello @Thw0rted

We are aware of the noise related to this rule and the good news is that we are currently working on a new set of rules about regular expressions (for Java at beginning)

The new security-rule (S5858) will be much more specific and will only raise when patterns known to have performance issues, such as nested quantifiers, are detected.

Eric

4

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.