From 04c230b12899b1417c500dc3aff99569e91134cd Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Thu, 1 Sep 2022 09:17:08 +0200 Subject: [PATCH 1/2] Docs fixes --- .../CWE/CWE-625/PermissiveDotRegex.qhelp | 17 ++++---- .../CWE/CWE-625/PermissiveDotRegex.ql | 4 +- .../CWE/CWE-625/PermissiveDotRegexQuery.qll | 39 +++++++++++-------- .../Security/CWE/CWE-625/Regex.qll | 10 ++--- 4 files changed, 36 insertions(+), 34 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp index 998edb38c21..4eb777d5141 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp @@ -4,10 +4,10 @@ -

By default, "dot" (.) in regular expressions matches all characters except newline characters \n and -\r. 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.

+

By default, a "dot" (.) in a regular expression matches all characters except the new line characters \n and +\r. Regular expressions containing a dot can be bypassed with the characters \r(%0a) and +\n(%0d) 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.

 @@ -17,18 +17,15 @@ to address this vulnerability.

 -

The following examples show the bad case and the good case respectively. The bad methods show a regex pattern allowing -bypass. In the good 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. +

The following snippets show a vulnerable example and a secure example respectively. The bad methods show a regex pattern allowing +a bypass by using line break characters. In the good 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.

-
  • Lay0us1: - CVE 2022-22978: Authorization Bypass in RegexRequestMatcher. -
  • Apache Shiro: Address the RegexRequestMatcher issue in 1.9.1.
    • diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql index cfaf7e4a387..7319c75320a 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.ql +++ b/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 diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll index 5ad0ce34d0c..1eb9037e4e8 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll +++ b/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 serlvet 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,8 +61,8 @@ 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", @@ -70,7 +70,8 @@ class SpringUriInputAnnotation extends Annotation { } } -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() | ( diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/Regex.qll b/java/ql/src/experimental/Security/CWE/CWE-625/Regex.qll index f084cd1f0de..8287eb78fb2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/Regex.qll +++ b/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() { From 6ffaa6918a5c1209c2e7419f1e5faa31ba37d4fa Mon Sep 17 00:00:00 2001 From: Anders Schack-Mulligen Date: Tue, 6 Sep 2022 14:11:48 +0200 Subject: [PATCH 2/2] Apply suggestions from code review --- .../experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp | 2 +- .../Security/CWE/CWE-625/PermissiveDotRegexQuery.qll | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp index 4eb777d5141..cc7843e7455 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegex.qhelp @@ -4,7 +4,7 @@ -

    By default, a "dot" (.) in a regular expression matches all characters except the new line characters \n and +

    By default, a "dot" (.) in a regular expression matches all characters except the newline characters \n and \r. Regular expressions containing a dot can be bypassed with the characters \r(%0a) and \n(%0d) 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.

    diff --git a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll index 1eb9037e4e8..21a44ebbb8f 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-625/PermissiveDotRegexQuery.qll @@ -19,7 +19,7 @@ private class PermissiveDotStr extends StringLiteral { } } -/** Remote flow sources obtained from the URI of a serlvet request. */ +/** Remote flow sources obtained from the URI of a servlet request. */ private class GetServletUriSource extends SourceModelCsv { override predicate row(string row) { row =