Queries to detect ReDoS and Regex injections in Java

[ggolawski]

This PR contains 2 queries to detect the cases where user input is matched against malicious regex, which may lead to DoS.

ReDoS query detects the cases where user input is matched against the malicious regex, which is hardcoded as a string literal. For example:

public class ReDoS {
  private static String EXPONENTIAL_REGEX = "(a+)+";

  public void regexMatch(@RequestParam String input) {
    Pattern p = Pattern.compile(EXPONENTIAL_REGEX);
    Matcher m = p.matcher(input);
    boolean matches = m.matches();
  }
}

RegexInjection query detects the cases where user input is matched against regex, which is being built using user supplied input (without escaping). For example:

public boolean isUsernameInPassword(@RequestParam String username, @RequestParam String password) {
  Pattern p = Pattern.compile(username);
  Matcher m = p.matcher(password);
  return m.matches();
}

Apart from the above, the 2 queries work the same and re-use the same code.

The following cases are handled:

• Pattern.matches(regex, input)
• input.matches(pattern) (where input is String)
• pattern is created first (Pattern.compile(regex)) and then matcher (p.matcher(input))

Detecting the last case has some limitations:

• it doesn't work if pattern is created in one method and matcher in another one, e.g.: void match(Pattern p) { p.matcher(input); } void foo() { match(Pattern.compile(regex)); }, but it works correctly if pattern is a class field and matcher is created in method, e.g.: private Pattern p = Pattern.compile(regex); void foo() { p.matcher(input); }
• the issue is reported if Matcher object is created with user supplied input on malicious pattern. To actually trigger the DoS condition, actual matching (e.g. matcher.matches()) is required, but usually if matcher is created it will eventually be used.

The RegexInjection query raises the flag only if regex is created using user supplied input (regex injection) and it's being used to match against user supplied input.
I've been thinking if the query shouldn't raise a flag if regex injection occurs, without checking if the regex is being used to match against user supplied input. This could reveal more bugs, but would also increase the number of false positives.
Please let me know what do you think. I can change this query if you wish.

[JLLeitschuh]

Suggested change

	}
	}

EOL at EOF

[ggolawski]

I'm not sure about this.
This sample is included in the qhelp. Will this newline at the end look good in the rendered help? Other samples don't have newline at EOF.
@felicitymay, what is your view on this?

[JLLeitschuh]

In order to be considered a valid file on some operating systems, you need a EOL at EOF.

A text file, under unix, consists of a series of lines, each of which ends with a newline character (\n). A file that is not empty and does not end with a newline is therefore not a text file.
- https://unix.stackexchange.com/a/18789

[ggolawski]

It's fine with me to change this. But I would like to wait for @felicitymay opinion, because other examples don't have EOL at EOF.

[felicitymay]

My apologies for the delay in replying. I hadn't responded because I was hoping that someone more knowledgeable would answer you. (I'm a content writer, rather than a developer, and work in Windows where EOL and EOF don't really come up.)

While @JLLeitschuh is right to point out that it's normal behavior in linux to end files with an extra newline, I suspect that @ggolawski is right to suggest that for these examples the extra blank line will look strange when they're rendered. @Semmle/java does that sound right?

[JLLeitschuh]

Suggested change

	}
	}

EOL at EOF

[aschackmull]

These two queries are definitely something that we'll want to include in our default suite, and as you've probably noticed we already have ReDoS queries for C# and JavaScript (indeed it looks like you copy-pasted the regex-regexes from our C# query). There are a number of things I'd like to implement differently, so we can't merge this PR. However, there are certainly useful parts here, that I'm certain we'll be able to credit. I've made an issue for our follow-up #2804.

[pwntester]

Add split, replaceAll, replaceFirst?

[pwntester]

Would be nice to cover common utility libraries such as Apache Commons. e.g.

• https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/RegExUtils.html
• https://commons.apache.org/proper/commons-validator/apidocs/org/apache/commons/validator/routines/RegexValidator.html
• https://commons.apache.org/proper/commons-digester/commons-digester-3.0/apidocs/org/apache/commons/digester3/SimpleRegexMatcher.html
• https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/Validate.html#matchesPattern-java.lang.CharSequence-java.lang.String-
• ...

Also, any Spring related classes if any