Merge pull request #9873 from luchua-bc/java/permissive-dot-regex

Java: CWE-625 Query to detect regex dot bypass

java/ql/src/experimental/Security/CWE/CWE-601/SpringUrlRedirect.ql

@@ -11,7 +11,7 @@
  */
 
 import java
-import SpringUrlRedirect
+import experimental.semmle.code.java.security.SpringUrlRedirect
 import semmle.code.java.dataflow.FlowSources
 import semmle.code.java.controlflow.Guards
 import DataFlow::PathGraph

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

@@ -0,0 +1,32 @@
+String PROTECTED_PATTERN = "/protected/.*";
+String CONSTRAINT_PATTERN = "/protected/xyz\\.xml";
+
+// BAD: A string with line return e.g. `/protected/%0dxyz` can bypass the path check
+Pattern p = Pattern.compile(PROTECTED_PATTERN);
+Matcher m = p.matcher(path);
+
+// GOOD: A string with line return e.g. `/protected/%0dxyz` cannot bypass the path check
+Pattern p = Pattern.compile(PROTECTED_PATTERN, Pattern.DOTALL);
+Matcher m = p.matcher(path);
+
+// GOOD: Only a specific path can pass the validation
+Pattern p = Pattern.compile(CONSTRAINT_PATTERN);
+Matcher m = p.matcher(path);
+
+if (m.matches()) {
+    // Protected page - check access token and redirect to login page
+} else {
+    // Not protected page - render content
+}
+
+// BAD: A string with line return e.g. `/protected/%0axyz` can bypass the path check
+boolean matches = path.matches(PROTECTED_PATTERN);
+
+// BAD: A string with line return e.g. `/protected/%0axyz` can bypass the path check
+boolean matches = Pattern.matches(PROTECTED_PATTERN, path);
+
+if (matches) {
+    // Protected page - check access token and redirect to login page
+} else {
+    // Not protected page - render content
+}

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

@@ -0,0 +1,40 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>By default, "dot" (<code>.</code>) in regular expressions matches all characters except newline characters <code>\n</code> and
+<code>\r</code>. Regular expressions containing a dot can be bypassed with the characters \r(%0a) , \n(%0d) when the default regex
+matching implementations of Java are used. When regular expressions serve to match protected resource patterns to grant access
+to protected application resources, attackers can gain access to unauthorized paths.</p>
+</overview>
+
+<recommendation>
+<p>To guard against unauthorized access, it is advisable to properly specify regex patterns for validating user input. The Java
+Pattern Matcher API <code>Pattern.compile(PATTERN, Pattern.DOTALL)</code> with the <code>DOTALL</code> flag set can be adopted
+to address this vulnerability.</p>
+</recommendation>
+
+<example>
+<p>The following examples show the bad case and the good case respectively. The <code>bad</code> methods show a regex pattern allowing
+bypass. In the <code>good</code> methods, it is shown how to solve this problem by either specifying the regex pattern correctly or
+use the Java API that can detect new line characters. 
+</p>
+
+<sample src="DotRegex.java" />
+</example>
+
+<references>
+<li>Lay0us1:
+  <a href="https://github.com/Lay0us1/CVE-2022-32532">CVE 2022-22978: Authorization Bypass in RegexRequestMatcher</a>.
+</li>
+<li>Apache Shiro:
+  <a href="https://github.com/apache/shiro/commit/6bcb92e06fa588b9c7790dd01bc02135d58d3f5b">Address the RegexRequestMatcher issue in 1.9.1</a>.
+</li>
+<li>CVE-2022-32532:
+  <a href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2022-32532">Applications using RegExPatternMatcher with "." in the regular expression are possibly vulnerable to an authorization bypass</a>.
+</li>
+</references>
+
+</qhelp>

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

@@ -0,0 +1,22 @@
+/**
+ * @name URL matched by permissive `.` in the regular expression
+ * @description URL validated with permissive `.` in regex  are possibly vulnerable
+ *              to an authorization bypass.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision high
+ * @id java/permissive-dot-regex
+ * @tags security
+ *       external/cwe-625
+ *       external/cwe-863
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import DataFlow::PathGraph
+import PermissiveDotRegexQuery
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, MatchRegexConfiguration conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potentially authentication bypass due to $@.",
+  source.getNode(), "user-provided value"

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

@@ -0,0 +1,218 @@
+/** 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 DataFlow2::Configuration {
+  PermissiveDotRegexConfig() { this = "PermissiveDotRegex::PermissiveDotRegexConfig" }
+
+  override predicate isSource(DataFlow2::Node src) { src.asExpr() instanceof PermissiveDotStr }
+
+  override predicate isSink(DataFlow2::Node sink) { sink instanceof CompileRegexSink }
+
+  override predicate isBarrier(DataFlow2::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)
+    ) and
+    exists(MethodAccess ma | any(PermissiveDotRegexConfig conf2).hasFlowToExpr(ma.getArgument(0)) |
+      // input.matches(regexPattern)
+      ma.getMethod() instanceof StringMatchMethod and
+      ma.getQualifier() = sink.asExpr()
+      or
+      // p = Pattern.compile(regexPattern); p.matcher(input)
+      ma.getMethod() instanceof PatternCompileMethod and
+      exists(MethodAccess 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
+      sink.asExpr() = ma.getArgument(1)
+    )
+  }
+}
+
+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

@@ -0,0 +1,50 @@
+/** Provides methods related to regular expression matching. */
+
+import java
+
+/**
+ * The class `Pattern` for pattern match.
+ */
+class Pattern extends RefType {
+  Pattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
+}
+
+/**
+ * The method `compile` for `Pattern`.
+ */
+class PatternCompileMethod extends Method {
+  PatternCompileMethod() {
+    this.getDeclaringType().getASupertype*() instanceof Pattern and
+    this.hasName("compile")
+  }
+}
+
+/**
+ * The method `matches` for `Pattern`.
+ */
+class PatternMatchMethod extends Method {
+  PatternMatchMethod() {
+    this.getDeclaringType().getASupertype*() instanceof Pattern and
+    this.hasName("matches")
+  }
+}
+
+/**
+ * The method `matcher` for `Pattern`.
+ */
+class PatternMatcherMethod extends Method {
+  PatternMatcherMethod() {
+    this.getDeclaringType().getASupertype*() instanceof Pattern and
+    this.hasName("matcher")
+  }
+}
+
+/**
+ * The method `matches` for `String`.
+ */
+class StringMatchMethod extends Method {
+  StringMatchMethod() {
+    this.getDeclaringType().getASupertype*() instanceof TypeString and
+    this.hasName("matches")
+  }
+}

java/ql/src/experimental/semmle/code/java/security/SpringUrlRedirect.qll

@@ -1,9 +1,7 @@
-import java
-import DataFlow
-import semmle.code.java.dataflow.FlowSources
-import semmle.code.java.dataflow.DataFlow2
-import semmle.code.java.dataflow.TaintTracking
-import semmle.code.java.frameworks.spring.SpringController
+/** Provides classes and predicates related to Spring URL redirect. */
+
+private import java
+private import semmle.code.java.dataflow.FlowSources
 
 /**
  * A concatenate expression using the string `redirect:` or `ajaxredirect:` or `forward:` on the left.
@@ -42,6 +40,13 @@ abstract class SpringUrlRedirectSink extends DataFlow::Node { }
  */
 private class SpringViewUrlRedirectSink extends SpringUrlRedirectSink {
   SpringViewUrlRedirectSink() {
+    // Hardcoded redirect such as "redirect:login"
+    this.asExpr()
+        .(CompileTimeConstantExpr)
+        .getStringValue()
+        .indexOf(["redirect:", "ajaxredirect:", "forward:"]) = 0 and
+    any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable())
+    or
     exists(RedirectBuilderExpr rbe |
       rbe.getRightOperand() = this.asExpr() and
       any(SpringRequestMappingMethod sqmm).polyCalls*(this.getEnclosingCallable())