Skip to content

Commit 8a73675

Browse files
authored
Merge pull request github#11070 from jcogs33/java-regex-injection
Java: Promote regex injection query from experimental
2 parents c2ac60f + 9e2ec9d commit 8a73675

File tree

17 files changed

+225
-245
lines changed

17 files changed

+225
-245
lines changed

java/ql/lib/semmle/code/java/frameworks/Regex.qll

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,27 @@
22

33
private import semmle.code.java.dataflow.ExternalFlow
44

5+
/** The class `java.util.regex.Pattern`. */
6+
class TypeRegexPattern extends Class {
7+
TypeRegexPattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
8+
}
9+
10+
/** The `quote` method of the `java.util.regex.Pattern` class. */
11+
class PatternQuoteMethod extends Method {
12+
PatternQuoteMethod() {
13+
this.getDeclaringType() instanceof TypeRegexPattern and
14+
this.hasName("quote")
15+
}
16+
}
17+
18+
/** The `LITERAL` field of the `java.util.regex.Pattern` class. */
19+
class PatternLiteralField extends Field {
20+
PatternLiteralField() {
21+
this.getDeclaringType() instanceof TypeRegexPattern and
22+
this.hasName("LITERAL")
23+
}
24+
}
25+
526
private class RegexModel extends SummaryModelCsv {
627
override predicate row(string s) {
728
s =

java/ql/lib/semmle/code/java/regex/RegexFlowModels.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ private class RegexSinkCsv extends SinkModelCsv {
2727
"com.google.common.base;Splitter;false;split;(CharSequence);;Argument[-1];regex-use[0];manual",
2828
"com.google.common.base;Splitter;false;splitToList;(CharSequence);;Argument[-1];regex-use[0];manual",
2929
"com.google.common.base;Splitter$MapSplitter;false;split;(CharSequence);;Argument[-1];regex-use[0];manual",
30+
"org.apache.commons.lang3;RegExUtils;false;removeAll;(String,String);;Argument[1];regex-use;manual",
31+
"org.apache.commons.lang3;RegExUtils;false;removeFirst;(String,String);;Argument[1];regex-use;manual",
32+
"org.apache.commons.lang3;RegExUtils;false;removePattern;(String,String);;Argument[1];regex-use;manual",
33+
"org.apache.commons.lang3;RegExUtils;false;replaceAll;(String,String,String);;Argument[1];regex-use;manual",
34+
"org.apache.commons.lang3;RegExUtils;false;replaceFirst;(String,String,String);;Argument[1];regex-use;manual",
35+
"org.apache.commons.lang3;RegExUtils;false;replacePattern;(String,String,String);;Argument[1];regex-use;manual",
3036
]
3137
}
3238
}
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/** Provides classes and predicates related to regex injection in Java. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.DataFlow
5+
private import semmle.code.java.frameworks.Regex
6+
private import semmle.code.java.regex.RegexFlowModels
7+
8+
/** A data flow sink for untrusted user input used to construct regular expressions. */
9+
abstract class RegexInjectionSink extends DataFlow::ExprNode { }
10+
11+
/** A sanitizer for untrusted user input used to construct regular expressions. */
12+
abstract class RegexInjectionSanitizer extends DataFlow::ExprNode { }
13+
14+
/** A method call that takes a regular expression as an argument. */
15+
private class DefaultRegexInjectionSink extends RegexInjectionSink {
16+
DefaultRegexInjectionSink() {
17+
// we only select sinks where there is direct regex creation, not regex uses
18+
sinkNode(this, ["regex-use[]", "regex-use[f1]", "regex-use[f-1]", "regex-use[-1]", "regex-use"])
19+
}
20+
}
21+
22+
/**
23+
* A call to the `Pattern.quote` method, which gives metacharacters or escape sequences
24+
* no special meaning.
25+
*/
26+
private class PatternQuoteCall extends RegexInjectionSanitizer {
27+
PatternQuoteCall() {
28+
exists(MethodAccess ma, Method m | m = ma.getMethod() |
29+
ma.getArgument(0) = this.asExpr() and
30+
m instanceof PatternQuoteMethod
31+
)
32+
}
33+
}
34+
35+
/**
36+
* Use of the `Pattern.LITERAL` flag with `Pattern.compile`, which gives metacharacters
37+
* or escape sequences no special meaning.
38+
*/
39+
private class PatternLiteralFlag extends RegexInjectionSanitizer {
40+
PatternLiteralFlag() {
41+
exists(MethodAccess ma, Method m, PatternLiteralField field | m = ma.getMethod() |
42+
ma.getArgument(0) = this.asExpr() and
43+
m.getDeclaringType() instanceof TypeRegexPattern and
44+
m.hasName("compile") and
45+
ma.getArgument(1) = field.getAnAccess()
46+
)
47+
}
48+
}
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/** Provides configurations to be used in queries related to regex injection. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.security.regexp.RegexInjection
7+
8+
/** A taint-tracking configuration for untrusted user input used to construct regular expressions. */
9+
class RegexInjectionConfiguration extends TaintTracking::Configuration {
10+
RegexInjectionConfiguration() { this = "RegexInjection" }
11+
12+
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
13+
14+
override predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink }
15+
16+
override predicate isSanitizer(DataFlow::Node node) { node instanceof RegexInjectionSanitizer }
17+
}
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
import java.util.regex.Pattern;
2+
import javax.servlet.http.HttpServlet;
3+
import javax.servlet.http.HttpServletRequest;
4+
5+
public class RegexInjectionDemo extends HttpServlet {
6+
7+
public boolean badExample(javax.servlet.http.HttpServletRequest request) {
8+
String regex = request.getParameter("regex");
9+
String input = request.getParameter("input");
10+
11+
// BAD: Unsanitized user input is used to construct a regular expression
12+
return input.matches(regex);
13+
}
14+
15+
public boolean goodExample(javax.servlet.http.HttpServletRequest request) {
16+
String regex = request.getParameter("regex");
17+
String input = request.getParameter("input");
18+
19+
// GOOD: User input is sanitized before constructing the regex
20+
return input.matches(Pattern.quote(regex));
21+
}
22+
}

java/ql/src/experimental/Security/CWE/CWE-730/RegexInjection.qhelp renamed to java/ql/src/Security/CWE/CWE-730/RegexInjection.qhelp

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,25 +15,25 @@ perform a Denial of Service attack.
1515
<recommendation>
1616
<p>
1717
Before embedding user input into a regular expression, use a sanitization function
18-
to escape meta-characters that have special meaning.
18+
such as <code>Pattern.quote</code> to escape meta-characters that have special meaning.
1919
</p>
2020
</recommendation>
2121

2222
<example>
2323
<p>
24-
The following example shows a HTTP request parameter that is used to construct a regular expression:
24+
The following example shows an HTTP request parameter that is used to construct a regular expression.
2525
</p>
26-
<sample src="RegexInjection.java" />
2726
<p>
2827
In the first case the user-provided regex is not escaped.
29-
If a malicious user provides a regex that has exponential worst case performance,
28+
If a malicious user provides a regex whose worst-case performance is exponential,
3029
then this could lead to a Denial of Service.
3130
</p>
3231
<p>
33-
In the second case, the user input is escaped using <code>escapeSpecialRegexChars</code> before being included
32+
In the second case, the user input is escaped using <code>Pattern.quote</code> before being included
3433
in the regular expression. This ensures that the user cannot insert characters which have a special
3534
meaning in regular expressions.
3635
</p>
36+
<sample src="RegexInjection.java" />
3737
</example>
3838

3939
<references>
@@ -44,5 +44,8 @@ OWASP:
4444
<li>
4545
Wikipedia: <a href="https://en.wikipedia.org/wiki/ReDoS">ReDoS</a>.
4646
</li>
47+
<li>
48+
Java API Specification: <a href="https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/regex/Pattern.html#quote(java.lang.String)">Pattern.quote</a>.
49+
</li>
4750
</references>
4851
</qhelp>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/**
2+
* @name Regular expression injection
3+
* @description User input should not be used in regular expressions without first being escaped,
4+
* otherwise a malicious user may be able to provide a regex that could require
5+
* exponential time on certain inputs.
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @security-severity 7.5
9+
* @precision high
10+
* @id java/regex-injection
11+
* @tags security
12+
* external/cwe/cwe-730
13+
* external/cwe/cwe-400
14+
*/
15+
16+
import java
17+
import semmle.code.java.security.regexp.RegexInjectionQuery
18+
import DataFlow::PathGraph
19+
20+
from DataFlow::PathNode source, DataFlow::PathNode sink, RegexInjectionConfiguration c
21+
where c.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink, "This regular expression is constructed from a $@.",
23+
source.getNode(), "user-provided value"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* The query, `java/regex-injection`, has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @edvraa](https://github.com/github/codeql/pull/5704).

java/ql/src/experimental/Security/CWE/CWE-730/RegexInjection.java

Lines changed: 0 additions & 38 deletions
This file was deleted.

java/ql/src/experimental/Security/CWE/CWE-730/RegexInjection.ql

Lines changed: 0 additions & 89 deletions
This file was deleted.

0 commit comments

Comments
 (0)