Skip to content

Java: Promote regex injection query from experimental #11070

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 23 commits into from
Nov 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions java/ql/lib/semmle/code/java/frameworks/Regex.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,27 @@

private import semmle.code.java.dataflow.ExternalFlow

/** The class `java.util.regex.Pattern`. */
class TypeRegexPattern extends Class {
TypeRegexPattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
}

/** The `quote` method of the `java.util.regex.Pattern` class. */
class PatternQuoteMethod extends Method {
PatternQuoteMethod() {
this.getDeclaringType() instanceof TypeRegexPattern and
this.hasName("quote")
}
}

/** The `LITERAL` field of the `java.util.regex.Pattern` class. */
class PatternLiteralField extends Field {
PatternLiteralField() {
this.getDeclaringType() instanceof TypeRegexPattern and
this.hasName("LITERAL")
}
}

private class RegexModel extends SummaryModelCsv {
override predicate row(string s) {
s =
Expand Down
6 changes: 6 additions & 0 deletions java/ql/lib/semmle/code/java/regex/RegexFlowModels.qll
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ private class RegexSinkCsv extends SinkModelCsv {
"com.google.common.base;Splitter;false;split;(CharSequence);;Argument[-1];regex-use[0];manual",
"com.google.common.base;Splitter;false;splitToList;(CharSequence);;Argument[-1];regex-use[0];manual",
"com.google.common.base;Splitter$MapSplitter;false;split;(CharSequence);;Argument[-1];regex-use[0];manual",
"org.apache.commons.lang3;RegExUtils;false;removeAll;(String,String);;Argument[1];regex-use;manual",
"org.apache.commons.lang3;RegExUtils;false;removeFirst;(String,String);;Argument[1];regex-use;manual",
"org.apache.commons.lang3;RegExUtils;false;removePattern;(String,String);;Argument[1];regex-use;manual",
"org.apache.commons.lang3;RegExUtils;false;replaceAll;(String,String,String);;Argument[1];regex-use;manual",
"org.apache.commons.lang3;RegExUtils;false;replaceFirst;(String,String,String);;Argument[1];regex-use;manual",
"org.apache.commons.lang3;RegExUtils;false;replacePattern;(String,String,String);;Argument[1];regex-use;manual",
]
}
}
48 changes: 48 additions & 0 deletions java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/** Provides classes and predicates related to regex injection in Java. */

import java
private import semmle.code.java.dataflow.DataFlow
private import semmle.code.java.frameworks.Regex
private import semmle.code.java.regex.RegexFlowModels

/** A data flow sink for untrusted user input used to construct regular expressions. */
abstract class RegexInjectionSink extends DataFlow::ExprNode { }

/** A sanitizer for untrusted user input used to construct regular expressions. */
abstract class RegexInjectionSanitizer extends DataFlow::ExprNode { }

/** A method call that takes a regular expression as an argument. */
private class DefaultRegexInjectionSink extends RegexInjectionSink {
DefaultRegexInjectionSink() {
// we only select sinks where there is direct regex creation, not regex uses
sinkNode(this, ["regex-use[]", "regex-use[f1]", "regex-use[f-1]", "regex-use[-1]", "regex-use"])
}
}

/**
* A call to the `Pattern.quote` method, which gives metacharacters or escape sequences
* no special meaning.
*/
private class PatternQuoteCall extends RegexInjectionSanitizer {
PatternQuoteCall() {
exists(MethodAccess ma, Method m | m = ma.getMethod() |
ma.getArgument(0) = this.asExpr() and
m instanceof PatternQuoteMethod
)
}
}

/**
* Use of the `Pattern.LITERAL` flag with `Pattern.compile`, which gives metacharacters
* or escape sequences no special meaning.
*/
private class PatternLiteralFlag extends RegexInjectionSanitizer {
PatternLiteralFlag() {
exists(MethodAccess ma, Method m, PatternLiteralField field | m = ma.getMethod() |
ma.getArgument(0) = this.asExpr() and
m.getDeclaringType() instanceof TypeRegexPattern and
m.hasName("compile") and
ma.getArgument(1) = field.getAnAccess()
)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/** Provides configurations to be used in queries related to regex injection. */

import java
import semmle.code.java.dataflow.FlowSources
import semmle.code.java.dataflow.TaintTracking
import semmle.code.java.security.regexp.RegexInjection

/** A taint-tracking configuration for untrusted user input used to construct regular expressions. */
class RegexInjectionConfiguration extends TaintTracking::Configuration {
RegexInjectionConfiguration() { this = "RegexInjection" }

override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }

override predicate isSink(DataFlow::Node sink) { sink instanceof RegexInjectionSink }

override predicate isSanitizer(DataFlow::Node node) { node instanceof RegexInjectionSanitizer }
}
22 changes: 22 additions & 0 deletions java/ql/src/Security/CWE/CWE-730/RegexInjection.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import java.util.regex.Pattern;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;

public class RegexInjectionDemo extends HttpServlet {

public boolean badExample(javax.servlet.http.HttpServletRequest request) {
String regex = request.getParameter("regex");
String input = request.getParameter("input");

// BAD: Unsanitized user input is used to construct a regular expression
return input.matches(regex);
}

public boolean goodExample(javax.servlet.http.HttpServletRequest request) {
String regex = request.getParameter("regex");
String input = request.getParameter("input");

// GOOD: User input is sanitized before constructing the regex
return input.matches(Pattern.quote(regex));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,25 +15,25 @@ perform a Denial of Service attack.
<recommendation>
<p>
Before embedding user input into a regular expression, use a sanitization function
to escape meta-characters that have special meaning.
such as <code>Pattern.quote</code> to escape meta-characters that have special meaning.
</p>
</recommendation>

<example>
<p>
The following example shows a HTTP request parameter that is used to construct a regular expression:
The following example shows an HTTP request parameter that is used to construct a regular expression.
</p>
<sample src="RegexInjection.java" />
<p>
In the first case the user-provided regex is not escaped.
If a malicious user provides a regex that has exponential worst case performance,
If a malicious user provides a regex whose worst-case performance is exponential,
then this could lead to a Denial of Service.
</p>
<p>
In the second case, the user input is escaped using <code>escapeSpecialRegexChars</code> before being included
In the second case, the user input is escaped using <code>Pattern.quote</code> before being included
in the regular expression. This ensures that the user cannot insert characters which have a special
meaning in regular expressions.
</p>
<sample src="RegexInjection.java" />
</example>

<references>
Expand All @@ -44,5 +44,8 @@ OWASP:
<li>
Wikipedia: <a href="https://en.wikipedia.org/wiki/ReDoS">ReDoS</a>.
</li>
<li>
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>.
</li>
</references>
</qhelp>
23 changes: 23 additions & 0 deletions java/ql/src/Security/CWE/CWE-730/RegexInjection.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/**
* @name Regular expression injection
* @description User input should not be used in regular expressions without first being escaped,
* otherwise a malicious user may be able to provide a regex that could require
* exponential time on certain inputs.
* @kind path-problem
* @problem.severity error
* @security-severity 7.5
* @precision high
* @id java/regex-injection
* @tags security
* external/cwe/cwe-730
* external/cwe/cwe-400
*/

import java
import semmle.code.java.security.regexp.RegexInjectionQuery
import DataFlow::PathGraph

from DataFlow::PathNode source, DataFlow::PathNode sink, RegexInjectionConfiguration c
where c.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "This regular expression is constructed from a $@.",
source.getNode(), "user-provided value"
4 changes: 4 additions & 0 deletions java/ql/src/change-notes/2022-11-03-regex-injection.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: newQuery
---
* 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).

This file was deleted.

89 changes: 0 additions & 89 deletions java/ql/src/experimental/Security/CWE/CWE-730/RegexInjection.ql

This file was deleted.

Loading