Optimize the query

java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql

@@ -12,177 +12,14 @@
  */
 
 import java
-import experimental.semmle.code.java.security.SpringUrlRedirect
-import semmle.code.java.controlflow.Guards
-import semmle.code.java.dataflow.ExternalFlow
 import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.security.UrlRedirect
 import DataFlow::PathGraph
-import Regex
+import PermissiveDotRegexQuery
 
-/** Source model of remote flow source with servlets. */
-private class GetServletUriSource extends SourceModelCsv {
-  override predicate row(string row) {
-    row =
-      [
-        "javax.servlet.http;HttpServletRequest;false;getPathInfo;();;ReturnValue;uri-path;manual",
-        "javax.servlet.http;HttpServletRequest;false;getPathTranslated;();;ReturnValue;uri-path;manual",
-        "javax.servlet.http;HttpServletRequest;false;getRequestURI;();;ReturnValue;uri-path;manual",
-        "javax.servlet.http;HttpServletRequest;false;getRequestURL;();;ReturnValue;uri-path;manual",
-        "javax.servlet.http;HttpServletRequest;false;getServletPath;();;ReturnValue;uri-path;manual"
-      ]
-  }
-}
-
-/** Sink model of servlet dispatcher. */
-private class UrlDispatchSink extends SinkModelCsv {
-  override predicate row(string row) {
-    row =
-      [
-        "javax.servlet;RequestDispatcher;false;forward;;;Argument[-1];url-dispatch;manual",
-        "javax.servlet;RequestDispatcher;false;include;;;Argument[-1];url-dispatch;manual"
-      ]
-  }
-}
-
-/** Sink model of servlet filter. */
-private class UrlFilterSink extends SinkModelCsv {
-  override predicate row(string row) {
-    row = ["javax.servlet;FilterChain;true;doFilter;;;Argument[-1];url-filter;manual"]
-  }
-}
-
-/** A Spring framework annotation indicating remote uri user input. */
-class SpringUriInputAnnotation extends Annotation {
-  SpringUriInputAnnotation() {
-    exists(AnnotationType a |
-      a = this.getType() and
-      a.getPackage().getName() = "org.springframework.web.bind.annotation"
-    |
-      (
-        a.hasName("PathVariable") or
-        a.hasName("RequestParam")
-      )
-    )
-  }
-}
-
-class SpringUriInputParameterSource extends DataFlow::Node {
-  SpringUriInputParameterSource() {
-    this.asParameter() =
-      any(SpringRequestMappingParameter srmp |
-        srmp.getAnAnnotation() instanceof SpringUriInputAnnotation
-      )
-  }
-}
-
-/**
- * `.` without a `\` prefix, which is likely not a character literal in regex
- */
-class PermissiveDotStr extends StringLiteral {
-  PermissiveDotStr() {
-    // Find `.` in a string that is not prefixed with `\` and ends with `.*` (no suffix like file extension)
-    exists(string s, int i | this.getValue() = s |
-      s.indexOf(".*") = i and
-      not s.charAt(i - 1) = "\\" and
-      s.length() = i + 2
-    )
-  }
-}
-
-/**
- * Permissive `.` in a regular expression.
- */
-class PermissiveDotEx extends Expr {
-  PermissiveDotEx() { this instanceof PermissiveDotStr }
-}
-
-/**
- * A data flow sink to construct regular expressions.
- */
-class CompileRegexSink extends DataFlow::ExprNode {
-  CompileRegexSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      (
-        ma.getArgument(0) = this.asExpr() and
-        (
-          m instanceof StringMatchMethod // input.matches(regexPattern)
-          or
-          m instanceof PatternCompileMethod // p = Pattern.compile(regexPattern)
-          or
-          m instanceof PatternMatchMethod // p = Pattern.matches(regexPattern, input)
-        )
-      )
-    )
-  }
-}
-
-/**
- * A flow configuration for permissive dot regex.
- */
-class PermissiveDotRegexConfig extends DataFlow::Configuration {
-  PermissiveDotRegexConfig() { this = "PermissiveDotRegex::PermissiveDotRegexConfig" }
-
-  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof PermissiveDotEx }
-
-  override predicate isSink(DataFlow::Node sink) { sink instanceof CompileRegexSink }
-
-  override predicate isBarrier(DataFlow::Node node) {
-    exists(
-      MethodAccess ma, Field f // Pattern.compile(PATTERN, Pattern.DOTALL)
-    |
-      ma.getMethod() instanceof PatternCompileMethod and
-      ma.getArgument(1) = f.getAnAccess() and
-      f.hasName("DOTALL") and
-      f.getDeclaringType() instanceof Pattern and
-      node.asExpr() = ma.getArgument(0)
-    )
-  }
-}
-
-/**
- * A taint-tracking configuration for untrusted user input used to match regular expressions.
- */
-class MatchRegexConfiguration extends TaintTracking::Configuration {
-  MatchRegexConfiguration() { this = "PermissiveDotRegex::MatchRegexConfiguration" }
-
-  override predicate isSource(DataFlow::Node source) {
-    sourceNode(source, "uri-path") or // Servlet uri source
-    source instanceof SpringUriInputParameterSource // Spring uri source
-  }
-
-  override predicate isSink(DataFlow::Node sink) {
-    sink instanceof MatchRegexSink and
-    exists(
-      Guard guard, Expr se, Expr ce // used in a condition to control url redirect, which is a typical security enforcement
-    |
-      (
-        sink.asExpr() = ce.(MethodAccess).getQualifier() or
-        sink.asExpr() = ce.(MethodAccess).getAnArgument() or
-        sink.asExpr() = ce
-      ) and
-      (
-        DataFlow::localExprFlow(ce, guard.(MethodAccess).getQualifier()) or
-        DataFlow::localExprFlow(ce, guard.(MethodAccess).getAnArgument())
-      ) and
-      (
-        DataFlow::exprNode(se) instanceof UrlRedirectSink or
-        sinkNode(DataFlow::exprNode(se), "url-dispatch") or
-        sinkNode(DataFlow::exprNode(se), "url-filter") or
-        DataFlow::exprNode(se) instanceof SpringUrlRedirectSink
-      ) and
-      guard.controls(se.getBasicBlock(), true)
-    )
-  }
-}
-
-from
-  DataFlow::PathNode source, DataFlow::PathNode sink, MatchRegexConfiguration conf,
-  DataFlow::PathNode source2, DataFlow::PathNode sink2, PermissiveDotRegexConfig conf2
+from DataFlow::PathNode source, DataFlow::PathNode sink, MatchRegexConfiguration conf
 where
   conf.hasFlowPath(source, sink) and
-  conf2.hasFlowPath(source2, sink2) and
-  exists(MethodAccess ma | ma.getArgument(0) = sink2.getNode().asExpr() |
+  exists(MethodAccess ma | any(PermissiveDotRegexConfig conf2).hasFlowToExpr(ma.getArgument(0)) |
     // input.matches(regexPattern)
     ma.getMethod() instanceof StringMatchMethod and
     ma.getQualifier() = sink.getNode().asExpr()

java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll

@@ -0,0 +1,201 @@
+/** Provides classes related to security-centered regular expression matching. */
+
+import java
+private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSources
+import experimental.semmle.code.java.security.SpringUrlRedirect
+import semmle.code.java.controlflow.Guards
+import semmle.code.java.security.UrlRedirect
+import Regex
+
+/** A string that ends with `.*` not prefixed with `\`. */
+class PermissiveDotStr extends StringLiteral {
+  PermissiveDotStr() {
+    exists(string s, int i | this.getValue() = s |
+      s.indexOf(".*") = i and
+      not s.charAt(i - 1) = "\\" and
+      s.length() = i + 2
+    )
+  }
+}
+
+/** Source model of remote flow source with servlets. */
+private class GetServletUriSource extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "javax.servlet.http;HttpServletRequest;false;getPathInfo;();;ReturnValue;uri-path;manual",
+        "javax.servlet.http;HttpServletRequest;false;getPathTranslated;();;ReturnValue;uri-path;manual",
+        "javax.servlet.http;HttpServletRequest;false;getRequestURI;();;ReturnValue;uri-path;manual",
+        "javax.servlet.http;HttpServletRequest;false;getRequestURL;();;ReturnValue;uri-path;manual",
+        "javax.servlet.http;HttpServletRequest;false;getServletPath;();;ReturnValue;uri-path;manual"
+      ]
+  }
+}
+
+/** Sink of servlet dispatcher. */
+private class UrlDispatchSink extends UrlRedirectSink {
+  UrlDispatchSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof RequestDispatchMethod and
+      this.asExpr() = ma.getQualifier()
+    )
+  }
+}
+
+/** The `doFilter` method of `javax.servlet.FilterChain`. */
+private class ServletFilterMethod extends Method {
+  ServletFilterMethod() {
+    this.getDeclaringType().getASupertype*().hasQualifiedName("javax.servlet", "FilterChain") and
+    this.hasName("doFilter")
+  }
+}
+
+/** Sink of servlet filter. */
+private class UrlFilterSink extends UrlRedirectSink {
+  UrlFilterSink() {
+    exists(MethodAccess ma |
+      ma.getMethod() instanceof ServletFilterMethod and
+      this.asExpr() = ma.getQualifier()
+    )
+  }
+}
+
+/** A Spring framework annotation indicating remote uri user input. */
+class SpringUriInputAnnotation extends Annotation {
+  SpringUriInputAnnotation() {
+    this.getType()
+        .hasQualifiedName("org.springframework.web.bind.annotation",
+          ["PathVariable", "RequestParam"])
+  }
+}
+
+class SpringUriInputParameterSource extends DataFlow::Node {
+  SpringUriInputParameterSource() {
+    this.asParameter() =
+      any(SpringRequestMappingParameter srmp |
+        srmp.getAnAnnotation() instanceof SpringUriInputAnnotation
+      )
+  }
+}
+
+/**
+ * A data flow sink to construct regular expressions.
+ */
+class CompileRegexSink extends DataFlow::ExprNode {
+  CompileRegexSink() {
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      (
+        ma.getArgument(0) = this.asExpr() and
+        (
+          m instanceof StringMatchMethod // input.matches(regexPattern)
+          or
+          m instanceof PatternCompileMethod // p = Pattern.compile(regexPattern)
+          or
+          m instanceof PatternMatchMethod // p = Pattern.matches(regexPattern, input)
+        )
+      )
+    )
+  }
+}
+
+/**
+ * A flow configuration for permissive dot regex.
+ */
+class PermissiveDotRegexConfig extends DataFlow::Configuration {
+  PermissiveDotRegexConfig() { this = "PermissiveDotRegex::PermissiveDotRegexConfig" }
+
+  override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof PermissiveDotStr }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof CompileRegexSink }
+
+  override predicate isBarrier(DataFlow::Node node) {
+    exists(
+      MethodAccess ma, Field f // Pattern.compile(PATTERN, Pattern.DOTALL)
+    |
+      ma.getMethod() instanceof PatternCompileMethod and
+      ma.getArgument(1) = f.getAnAccess() and
+      f.hasName("DOTALL") and
+      f.getDeclaringType() instanceof Pattern and
+      node.asExpr() = ma.getArgument(0)
+    )
+  }
+}
+
+/**
+ * A taint-tracking configuration for untrusted user input used to match regular expressions.
+ */
+class MatchRegexConfiguration extends TaintTracking::Configuration {
+  MatchRegexConfiguration() { this = "PermissiveDotRegex::MatchRegexConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    sourceNode(source, "uri-path") or // Servlet uri source
+    source instanceof SpringUriInputParameterSource // Spring uri source
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink instanceof MatchRegexSink and
+    exists(
+      Guard guard, Expr se, Expr ce // used in a condition to control url redirect, which is a typical security enforcement
+    |
+      (
+        sink.asExpr() = ce.(MethodAccess).getQualifier() or
+        sink.asExpr() = ce.(MethodAccess).getAnArgument() or
+        sink.asExpr() = ce
+      ) and
+      (
+        DataFlow::localExprFlow(ce, guard.(MethodAccess).getQualifier()) or
+        DataFlow::localExprFlow(ce, guard.(MethodAccess).getAnArgument())
+      ) and
+      (
+        DataFlow::exprNode(se) instanceof UrlRedirectSink or
+        DataFlow::exprNode(se) instanceof SpringUrlRedirectSink
+      ) and
+      guard.controls(se.getBasicBlock(), true)
+    )
+  }
+}
+
+abstract class MatchRegexSink extends DataFlow::ExprNode { }
+
+/**
+ * A data flow sink to string match regular expressions.
+ */
+class StringMatchRegexSink extends MatchRegexSink {
+  StringMatchRegexSink() {
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      (
+        m instanceof StringMatchMethod and
+        ma.getQualifier() = this.asExpr()
+      )
+    )
+  }
+}
+
+/**
+ * A data flow sink to `pattern.matches` regular expressions.
+ */
+class PatternMatchRegexSink extends MatchRegexSink {
+  PatternMatchRegexSink() {
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      (
+        m instanceof PatternMatchMethod and
+        ma.getArgument(1) = this.asExpr()
+      )
+    )
+  }
+}
+
+/**
+ * A data flow sink to `pattern.matcher` match regular expressions.
+ */
+class PatternMatcherRegexSink extends MatchRegexSink {
+  PatternMatcherRegexSink() {
+    exists(MethodAccess ma, Method m | m = ma.getMethod() |
+      (
+        m instanceof PatternMatcherMethod and
+        ma.getArgument(0) = this.asExpr()
+      )
+    )
+  }
+}

java/ql/src/experimental/Security/CWE/CWE-625/Regex.qll

@@ -1,19 +1,16 @@
 /** Provides methods related to regular expression matching. */
 
 import java
-private import semmle.code.java.dataflow.ExternalFlow
-private import semmle.code.java.dataflow.FlowSources
-private import semmle.code.java.dataflow.TaintTracking2
 
 /**
- * The  class `Pattern` for pattern match.
+ * The class `Pattern` for pattern match.
  */
 class Pattern extends RefType {
   Pattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
 }
 
 /**
- * The  method `compile` for `Pattern`.
+ * The method `compile` for `Pattern`.
  */
 class PatternCompileMethod extends Method {
   PatternCompileMethod() {
@@ -23,7 +20,7 @@ class PatternCompileMethod extends Method {
 }
 
 /**
- * The  method `matches` for `Pattern`.
+ * The method `matches` for `Pattern`.
  */
 class PatternMatchMethod extends Method {
   PatternMatchMethod() {
@@ -33,7 +30,7 @@ class PatternMatchMethod extends Method {
 }
 
 /**
- * The  method `matcher` for `Pattern`.
+ * The method `matcher` for `Pattern`.
  */
 class PatternMatcherMethod extends Method {
   PatternMatcherMethod() {
@@ -43,7 +40,7 @@ class PatternMatcherMethod extends Method {
 }
 
 /**
- * The  method `matches` for `String`.
+ * The method `matches` for `String`.
  */
 class StringMatchMethod extends Method {
   StringMatchMethod() {
@@ -51,47 +48,3 @@ class StringMatchMethod extends Method {
     this.hasName("matches")
   }
 }
-
-abstract class MatchRegexSink extends DataFlow::ExprNode { }
-
-/**
- * A data flow sink to string match regular expressions.
- */
-class StringMatchRegexSink extends MatchRegexSink {
-  StringMatchRegexSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      (
-        m instanceof StringMatchMethod and
-        ma.getQualifier() = this.asExpr()
-      )
-    )
-  }
-}
-
-/**
- * A data flow sink to `pattern.matches` regular expressions.
- */
-class PatternMatchRegexSink extends MatchRegexSink {
-  PatternMatchRegexSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      (
-        m instanceof PatternMatchMethod and
-        ma.getArgument(1) = this.asExpr()
-      )
-    )
-  }
-}
-
-/**
- * A data flow sink to `pattern.matcher` match regular expressions.
- */
-class PatternMatcherRegexSink extends MatchRegexSink {
-  PatternMatcherRegexSink() {
-    exists(MethodAccess ma, Method m | m = ma.getMethod() |
-      (
-        m instanceof PatternMatcherMethod and
-        ma.getArgument(0) = this.asExpr()
-      )
-    )
-  }
-}