Java: Add RegexMatch concept and recognise @Pattern annotation as sanitizer

java/ql/lib/java.qll

@@ -9,6 +9,7 @@ import semmle.code.Unit
 import semmle.code.java.Annotation
 import semmle.code.java.Compilation
 import semmle.code.java.CompilationUnit
+import semmle.code.java.Concepts
 import semmle.code.java.ControlFlowGraph
 import semmle.code.java.Dependency
 import semmle.code.java.Element

java/ql/lib/semmle/code/java/Concepts.qll

@@ -0,0 +1,88 @@
+/**
+ * Provides abstract classes representing generic concepts such as file system
+ * access or system command execution, for which individual framework libraries
+ * provide concrete subclasses.
+ */
+overlay[local?]
+module;
+
+import java
+private import semmle.code.java.dataflow.DataFlow
+
+/**
+ * A data-flow node that executes a regular expression.
+ *
+ * Extend this class to refine existing API models. If you want to model new APIs,
+ * extend `RegexExecution::Range` instead.
+ */
+class RegexExecution extends DataFlow::Node instanceof RegexExecution::Range {
+  /** Gets the data flow node for the regex being executed by this node. */
+  DataFlow::Node getRegex() { result = super.getRegex() }
+
+  /** Gets a dataflow node for the string to be searched or matched against. */
+  DataFlow::Node getString() { result = super.getString() }
+
+  /**
+   * Gets the name of this regex execution, typically the name of an executing method.
+   * This is used for nice alert messages and should include the module if possible.
+   */
+  string getName() { result = super.getName() }
+}
+
+/** Provides classes for modeling new regular-expression execution APIs. */
+module RegexExecution {
+  /**
+   * A data flow node that executes a regular expression.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `RegexExecution` instead.
+   */
+  abstract class Range extends DataFlow::Node {
+    /** Gets the data flow node for the regex being executed by this node. */
+    abstract DataFlow::Node getRegex();
+
+    /** Gets a data flow node for the string to be searched or matched against. */
+    abstract DataFlow::Node getString();
+
+    /**
+     * Gets the name of this regex execution, typically the name of an executing method.
+     * This is used for nice alert messages and should include the module if possible.
+     */
+    abstract string getName();
+  }
+
+  private class RangeFromExpr extends Range {
+    private RegexExecutionExpr::Range ree;
+
+    RangeFromExpr() { this.asExpr() = ree }
+
+    override DataFlow::Node getRegex() { result.asExpr() = ree.getRegex() }
+
+    override DataFlow::Node getString() { result.asExpr() = ree.getString() }
+
+    override string getName() { result = ree.getName() }
+  }
+}
+
+/** Provides classes for modeling new regular-expression execution APIs. */
+module RegexExecutionExpr {
+  /**
+   * An expression that executes a regular expression.
+   *
+   * Extend this class to model new APIs. If you want to refine existing API models,
+   * extend `RegexExecution` instead.
+   */
+  abstract class Range extends Expr {
+    /** Gets the expression for the regex being executed by this node. */
+    abstract Expr getRegex();
+
+    /** Gets a expression for the string to be searched or matched against. */
+    abstract Expr getString();
+
+    /**
+     * Gets the name of this regex execution, typically the name of an executing method.
+     * This is used for nice alert messages and should include the module if possible.
+     */
+    abstract string getName();
+  }
+}

java/ql/lib/semmle/code/java/JDK.qll

@@ -48,13 +48,19 @@ class StringContainsMethod extends Method {
 }
 
 /** A call to the `java.lang.String.matches` method. */
-class StringMatchesCall extends MethodCall {
+class StringMatchesCall extends MethodCall, RegexExecutionExpr::Range {
   StringMatchesCall() {
     exists(Method m | m = this.getMethod() |
       m.getDeclaringType() instanceof TypeString and
       m.hasName("matches")
     )
   }
+
+  override Expr getRegex() { result = this.getArgument(0) }
+
+  override Expr getString() { result = this.getQualifier() }
+
+  override string getName() { result = "String.matches" }
 }
 
 /** A call to the `java.lang.String.replaceAll` method. */

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

@@ -3,6 +3,7 @@
 module;
 
 import java
+private import semmle.code.java.dataflow.DataFlow
 
 /** The class `java.util.regex.Matcher`. */
 class TypeRegexMatcher extends Class {
@@ -24,6 +25,16 @@
   TypeRegexPattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
 }
 
+/**
+ * The `compile` method of `java.util.regex.Pattern`.
+ */
+class PatternCompileMethod extends Method {
+  PatternCompileMethod() {
+    this.getDeclaringType() instanceof TypeRegexPattern and
+    this.hasName("compile")
+  }
+}
+
 /**
  * The `matches` method of `java.util.regex.Pattern`.
  */
@@ -59,3 +70,43 @@
     this.hasName("LITERAL")
   }
 }
+
+/** A call to the `compile` method of `java.util.regex.Pattern` */
+class PatternCompileCall extends MethodCall {
+  PatternCompileCall() { this.getMethod() instanceof PatternCompileMethod }
+}
+
+/** A call to the `matcher` method of `java.util.regex.Pattern` */
+class PatternMatcherCall extends MethodCall {
+  PatternMatcherCall() { this.getMethod() instanceof PatternMatcherMethod }
+}
+
+/** A call to the `matches` method of `java.util.regex.Pattern` */
+class PatternMatchesCall extends MethodCall, RegexExecutionExpr::Range {
+  PatternMatchesCall() { this.getMethod() instanceof PatternMatchesMethod }
+
+  override Expr getRegex() { result = this.getArgument(0) }
+
+  override Expr getString() { result = this.getArgument(1) }
+
+  override string getName() { result = "Pattern.matches" }
+}
+
+/** A call to the `matches` method of `java.util.regex.Matcher` */
+class MatcherMatchesCall extends MethodCall, RegexExecutionExpr::Range {
+  MatcherMatchesCall() { this.getMethod() instanceof MatcherMatchesMethod }
+
+  PatternMatcherCall getPatternMatcherCall() {
+    DataFlow::localExprFlow(result, this.getQualifier())
+  }
+
+  PatternCompileCall getPatternCompileCall() {
+    DataFlow::localExprFlow(result, this.getPatternMatcherCall())
+  }
+
+  override Expr getRegex() { result = this.getPatternCompileCall().getArgument(0) }
+
+  override Expr getString() { result = this.getPatternMatcherCall().getArgument(0) }
+
+  override string getName() { result = "Matcher.matches" }
+}

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

@@ -427,20 +427,17 @@ private class ReplaceDirectoryCharactersSanitizer extends StringReplaceOrReplace
   }
 }
 
-/** Holds if `target` is the first argument of `matchesCall`. */
-private predicate isMatchesTarget(StringMatchesCall matchesCall, CompileTimeConstantExpr target) {
-  target = matchesCall.getArgument(0)
-}
-
 /**
  * Holds if `matchesCall` confirms that `checkedExpr` does not contain any directory characters
  * on the given `branch`.
  */
-private predicate isMatchesCall(StringMatchesCall matchesCall, Expr checkedExpr, boolean branch) {
+private predicate isMatchesCall(
+  RegexExecutionExpr::Range regexMatch, Expr checkedExpr, boolean branch
+) {
   exists(CompileTimeConstantExpr target, string targetValue |
-    isMatchesTarget(matchesCall, target) and
+    target = regexMatch.getRegex() and
     target.getStringValue() = targetValue and
-    checkedExpr = matchesCall.getQualifier()
+    checkedExpr = regexMatch.getString()
   |
     (
       // Allow anything except `.`, '/', '\'

java/ql/lib/semmle/code/java/security/Sanitizers.qll

@@ -46,19 +46,11 @@ predicate regexpMatchGuardChecks(Guard guard, Expr e, boolean branch) {
     guard = mc and
     branch = true
   |
-    // `String.matches` and other `matches` methods.
+    e = mc.(RegexExecutionExpr::Range).getString()
+    or
+    // Other `matches` methods.
     method.getName() = "matches" and
     e = mc.getQualifier()
-    or
-    method instanceof PatternMatchesMethod and
-    e = mc.getArgument(1)
-    or
-    method instanceof MatcherMatchesMethod and
-    exists(MethodCall matcherCall |
-      matcherCall.getMethod() instanceof PatternMatcherMethod and
-      e = matcherCall.getArgument(0) and
-      DataFlow::localExprFlow(matcherCall, mc.getQualifier())
-    )
   )
 }
 

java/ql/lib/semmle/code/java/security/regexp/RegexInjection.qll

@@ -31,11 +31,9 @@ private class ExternalRegexInjectionSanitizer extends RegexInjectionSanitizer {
  */
 private class PatternLiteralFlag extends RegexInjectionSanitizer {
   PatternLiteralFlag() {
-    exists(MethodCall 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()
+    exists(PatternCompileCall pcc, PatternLiteralField field |
+      pcc.getArgument(0) = this.asExpr() and
+      pcc.getArgument(1) = field.getAnAccess()
     )
   }
 }

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

@@ -7,7 +7,7 @@ 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
+private import semmle.code.java.frameworks.Regex
 
 overlay[local?]
 private class ActivateModels extends ActiveExperimentalModels {
@@ -81,11 +81,11 @@ private class CompileRegexSink extends DataFlow::ExprNode {
       (
         ma.getArgument(0) = this.asExpr() and
         (
-          m instanceof StringMatchMethod // input.matches(regexPattern)
+          ma instanceof StringMatchesCall // input.matches(regexPattern)
           or
-          m instanceof PatternCompileMethod // p = Pattern.compile(regexPattern)
+          ma instanceof PatternCompileCall // p = Pattern.compile(regexPattern)
           or
-          m instanceof PatternMatchMethod // p = Pattern.matches(regexPattern, input)
+          ma instanceof PatternMatchesCall // p = Pattern.matches(regexPattern, input)
         )
       )
     )
@@ -107,7 +107,7 @@ private module PermissiveDotRegexConfig implements DataFlow::ConfigSig {
       ma.getMethod() instanceof PatternCompileMethod and
       ma.getArgument(1) = f.getAnAccess() and
       f.hasName("DOTALL") and
-      f.getDeclaringType() instanceof Pattern and
+      f.getDeclaringType() instanceof TypeRegexPattern and
       node.asExpr() = ma.getArgument(0)
     )
   }
@@ -147,19 +147,19 @@ module MatchRegexConfig implements DataFlow::ConfigSig {
     ) and
     exists(MethodCall ma | PermissiveDotRegexFlow::flowToExpr(ma.getArgument(0)) |
       // input.matches(regexPattern)
-      ma.getMethod() instanceof StringMatchMethod and
+      ma instanceof StringMatchesCall and
       ma.getQualifier() = sink.asExpr()
       or
       // p = Pattern.compile(regexPattern); p.matcher(input)
-      ma.getMethod() instanceof PatternCompileMethod and
+      ma instanceof PatternCompileCall and
       exists(MethodCall pma |
         pma.getMethod() instanceof PatternMatcherMethod and
         sink.asExpr() = pma.getArgument(0) and
         DataFlow::localExprFlow(ma, pma.getQualifier())
       )
       or
       // p = Pattern.matches(regexPattern, input)
-      ma.getMethod() instanceof PatternMatchMethod and
+      ma instanceof PatternMatchesCall and
       sink.asExpr() = ma.getArgument(1)
     )
   }
@@ -176,28 +176,14 @@ abstract class MatchRegexSink extends DataFlow::ExprNode { }
  * A string being matched against a regular expression.
  */
 private class StringMatchRegexSink extends MatchRegexSink {
-  StringMatchRegexSink() {
-    exists(MethodCall ma, Method m | m = ma.getMethod() |
-      (
-        m instanceof StringMatchMethod and
-        ma.getQualifier() = this.asExpr()
-      )
-    )
-  }
+  StringMatchRegexSink() { any(StringMatchesCall mc).getQualifier() = this.asExpr() }
 }
 
 /**
  * A string being matched against a regular expression using a pattern.
  */
 private class PatternMatchRegexSink extends MatchRegexSink {
-  PatternMatchRegexSink() {
-    exists(MethodCall ma, Method m | m = ma.getMethod() |
-      (
-        m instanceof PatternMatchMethod and
-        ma.getArgument(1) = this.asExpr()
-      )
-    )
-  }
+  PatternMatchRegexSink() { any(PatternMatchesCall mc).getArgument(1) = this.asExpr() }
 }
 
 /**