Merge pull request #10249 from atorralba/atorralba/regex-dot-bypass-docs

Java: Documentation fixes in the "Permissive dot regex" experimental query

Author: atorralba

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

@@ -4,10 +4,10 @@
 <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>
+<p>By default, a "dot" (<code>.</code>) in a regular expression matches all characters except the newline characters <code>\n</code> and
+<code>\r</code>. Regular expressions containing a dot can be bypassed with the characters <code>\r</code>(<code>%0a</code>) and 
+<code>\n</code>(<code>%0d</code>) when the default Java regular expression matching implementations are used. This becomes a security issue
+if these regular expressions are used to decide whether to grant access to protected application resources.</p>
 </overview>
 
 <recommendation>
@@ -17,18 +17,15 @@ 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>The following snippets show a vulnerable example and a secure example respectively. The <code>bad</code> methods show a regex pattern allowing
+a bypass by using line break characters. In the <code>good</code> methods, it is shown how to solve this problem by either specifying the regex 
+pattern correctly or using a Java API that properly matches 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>

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

@@ -1,6 +1,6 @@
 /**
- * @name URL matched by permissive `.` in the regular expression
- * @description URL validated with permissive `.` in regex  are possibly vulnerable
+ * @name URL matched by permissive `.` in a regular expression
+ * @description URLs validated with a permissive `.` in regular expressions may be vulnerable
  *              to an authorization bypass.
  * @kind path-problem
  * @problem.severity warning

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

@@ -9,7 +9,7 @@ import semmle.code.java.security.UrlRedirect
 import Regex
 
 /** A string that ends with `.*` not prefixed with `\`. */
-class PermissiveDotStr extends StringLiteral {
+private class PermissiveDotStr extends StringLiteral {
   PermissiveDotStr() {
     exists(string s, int i | this.getValue() = s |
       s.indexOf(".*") = i and
@@ -19,7 +19,7 @@ class PermissiveDotStr extends StringLiteral {
   }
 }
 
-/** Source model of remote flow source with servlets. */
+/** Remote flow sources obtained from the URI of a servlet request. */
 private class GetServletUriSource extends SourceModelCsv {
   override predicate row(string row) {
     row =
@@ -33,7 +33,7 @@ private class GetServletUriSource extends SourceModelCsv {
   }
 }
 
-/** Sink of servlet dispatcher. */
+/** The qualifier of a request dispatch method call. */
 private class UrlDispatchSink extends UrlRedirectSink {
   UrlDispatchSink() {
     exists(MethodAccess ma |
@@ -51,7 +51,7 @@ private class ServletFilterMethod extends Method {
   }
 }
 
-/** Sink of servlet filter. */
+/** The qualifier of a servlet filter method call. */
 private class UrlFilterSink extends UrlRedirectSink {
   UrlFilterSink() {
     exists(MethodAccess ma |
@@ -61,16 +61,17 @@ private class UrlFilterSink extends UrlRedirectSink {
   }
 }
 
-/** A Spring framework annotation indicating remote uri user input. */
-class SpringUriInputAnnotation extends Annotation {
+/** A Spring framework annotation indicating that a URI is user-provided. */
+private class SpringUriInputAnnotation extends Annotation {
   SpringUriInputAnnotation() {
     this.getType()
         .hasQualifiedName("org.springframework.web.bind.annotation",
           ["PathVariable", "RequestParam"])
   }
 }
 
-class SpringUriInputParameterSource extends DataFlow::Node {
+/** A user-provided URI parameter of a request mapping method. */
+private class SpringUriInputParameterSource extends DataFlow::Node {
   SpringUriInputParameterSource() {
     this.asParameter() =
       any(SpringRequestMappingParameter srmp |
@@ -82,7 +83,7 @@ class SpringUriInputParameterSource extends DataFlow::Node {
 /**
  * A data flow sink to construct regular expressions.
  */
-class CompileRegexSink extends DataFlow::ExprNode {
+private class CompileRegexSink extends DataFlow::ExprNode {
   CompileRegexSink() {
     exists(MethodAccess ma, Method m | m = ma.getMethod() |
       (
@@ -100,9 +101,9 @@ class CompileRegexSink extends DataFlow::ExprNode {
 }
 
 /**
- * A flow configuration for permissive dot regex.
+ * A data flow configuration for regular expressions that include permissive dots.
  */
-class PermissiveDotRegexConfig extends DataFlow2::Configuration {
+private class PermissiveDotRegexConfig extends DataFlow2::Configuration {
   PermissiveDotRegexConfig() { this = "PermissiveDotRegex::PermissiveDotRegexConfig" }
 
   override predicate isSource(DataFlow2::Node src) { src.asExpr() instanceof PermissiveDotStr }
@@ -123,7 +124,8 @@ class PermissiveDotRegexConfig extends DataFlow2::Configuration {
 }
 
 /**
- * A taint-tracking configuration for untrusted user input used to match regular expressions.
+ * A taint-tracking configuration for untrusted user input used to match regular expressions
+ * that include permissive dots.
  */
 class MatchRegexConfiguration extends TaintTracking::Configuration {
   MatchRegexConfiguration() { this = "PermissiveDotRegex::MatchRegexConfiguration" }
@@ -173,12 +175,15 @@ class MatchRegexConfiguration extends TaintTracking::Configuration {
   }
 }
 
+/**
+ * A data flow sink representing a string being matched against a regular expression.
+ */
 abstract class MatchRegexSink extends DataFlow::ExprNode { }
 
 /**
- * A data flow sink to string match regular expressions.
+ * A string being matched against a regular expression.
  */
-class StringMatchRegexSink extends MatchRegexSink {
+private class StringMatchRegexSink extends MatchRegexSink {
   StringMatchRegexSink() {
     exists(MethodAccess ma, Method m | m = ma.getMethod() |
       (
@@ -190,9 +195,9 @@ class StringMatchRegexSink extends MatchRegexSink {
 }
 
 /**
- * A data flow sink to `pattern.matches` regular expressions.
+ * A string being matched against a regular expression using a pattern.
  */
-class PatternMatchRegexSink extends MatchRegexSink {
+private class PatternMatchRegexSink extends MatchRegexSink {
   PatternMatchRegexSink() {
     exists(MethodAccess ma, Method m | m = ma.getMethod() |
       (
@@ -204,9 +209,9 @@ class PatternMatchRegexSink extends MatchRegexSink {
 }
 
 /**
- * A data flow sink to `pattern.matcher` match regular expressions.
+ * A string being used to create a pattern matcher.
  */
-class PatternMatcherRegexSink extends MatchRegexSink {
+private class PatternMatcherRegexSink extends MatchRegexSink {
   PatternMatcherRegexSink() {
     exists(MethodAccess ma, Method m | m = ma.getMethod() |
       (

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

@@ -3,14 +3,14 @@
 import java
 
 /**
- * The class `Pattern` for pattern match.
+ * The class `java.util.regex.Pattern`.
  */
 class Pattern extends RefType {
   Pattern() { this.hasQualifiedName("java.util.regex", "Pattern") }
 }
 
 /**
- * The method `compile` for `Pattern`.
+ * The method `compile` of `java.util.regex.Pattern`.
  */
 class PatternCompileMethod extends Method {
   PatternCompileMethod() {
@@ -20,7 +20,7 @@ class PatternCompileMethod extends Method {
 }
 
 /**
- * The method `matches` for `Pattern`.
+ * The method `matches` of `java.util.regex.Pattern`.
  */
 class PatternMatchMethod extends Method {
   PatternMatchMethod() {
@@ -30,7 +30,7 @@ class PatternMatchMethod extends Method {
 }
 
 /**
- * The method `matcher` for `Pattern`.
+ * The method `matcher` of `java.util.regex.Pattern`.
  */
 class PatternMatcherMethod extends Method {
   PatternMatcherMethod() {
@@ -40,7 +40,7 @@ class PatternMatcherMethod extends Method {
 }
 
 /**
- * The method `matches` for `String`.
+ * The method `matches` of `java.lang.String`.
  */
 class StringMatchMethod extends Method {
   StringMatchMethod() {