Merge pull request #21326 from owen-mc/java/log-injection-regex-match

Java: Recognise `@Pattern` annotation as sanitizer for log injection

java/ql/lib/change-notes/2026-02-14-pattern-annotation-log-injection-sanitizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Using a regular expression to check that a string doesn't contain any line breaks is already a sanitizer for `java/log-injection`. Additional ways of doing the regular expression check are now recognised, including annotation with `@javax.validation.constraints.Pattern`.

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

@@ -45,11 +45,11 @@ private class LineBreaksLogInjectionSanitizer extends LogInjectionSanitizer {
 }
 
 private predicate stringMethodCall(
-  MethodCall ma, CompileTimeConstantExpr arg0, CompileTimeConstantExpr arg1
+  MethodCall mc, CompileTimeConstantExpr arg0, CompileTimeConstantExpr arg1
 ) {
-  ma.getMethod().getDeclaringType() instanceof TypeString and
-  arg0 = ma.getArgument(0) and
-  arg1 = ma.getArgument(1)
+  mc.getMethod().getDeclaringType() instanceof TypeString and
+  arg0 = mc.getArgument(0) and
+  arg1 = mc.getArgument(1)
 }
 
 private predicate stringMethodArgument(CompileTimeConstantExpr arg) {
@@ -64,22 +64,23 @@ private predicate stringMethodArgumentValueMatches(CompileTimeConstantExpr const
 }
 
 /**
- * Holds if the return value of `ma` is sanitized against log injection attacks
- * by removing line breaks from it.
+ * Holds if `e` is sanitized against log injection attacks by removing line
+ * breaks from it.
  */
-private predicate logInjectionSanitizer(MethodCall ma) {
-  exists(CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
-    stringMethodCall(ma, target, replacement) and
+private predicate logInjectionSanitizer(Expr e) {
+  exists(MethodCall mc, CompileTimeConstantExpr target, CompileTimeConstantExpr replacement |
+    e = mc and
+    stringMethodCall(mc, target, replacement) and
     not stringMethodArgumentValueMatches(replacement, ["%\n%", "%\r%"])
   |
-    ma.getMethod().hasName("replace") and
+    mc.getMethod().hasName("replace") and
     not replacement.getIntValue() = [10, 13] and
     (
       target.getIntValue() = [10, 13] or // 10 == '\n', 13 == '\r'
       target.getStringValue() = ["\n", "\r"]
     )
     or
-    ma.getMethod().hasName("replaceAll") and
+    mc.getMethod().hasName("replaceAll") and
     (
       // Replace anything not in an allow list
       target.getStringValue().matches("[^%]") and
@@ -89,6 +90,13 @@ private predicate logInjectionSanitizer(MethodCall ma) {
       target.getStringValue() = ["\n", "\r", "\\n", "\\r", "\\R"]
     )
   )
+  or
+  exists(RegexMatch rm, CompileTimeConstantExpr target |
+    rm instanceof Annotation and
+    e = rm.getASanitizedExpr() and
+    target = rm.getRegex() and
+    regexPreventsLogInjection(target.getStringValue(), true)
+  )
 }
 
 /**
@@ -96,41 +104,44 @@ private predicate logInjectionSanitizer(MethodCall ma) {
  * by checking if there are line breaks in `e`.
  */
 private predicate logInjectionGuard(Guard g, Expr e, boolean branch) {
-  exists(MethodCall ma, CompileTimeConstantExpr target |
-    ma = g and
-    target = ma.getArgument(0)
-  |
-    ma.getMethod().getDeclaringType() instanceof TypeString and
-    ma.getMethod().hasName("contains") and
-    target.getStringValue() = ["\n", "\r"] and
-    e = ma.getQualifier() and
+  exists(MethodCall mc | mc = g |
+    mc.getMethod() instanceof StringContainsMethod and
+    mc.getArgument(0).(CompileTimeConstantExpr).getStringValue() = ["\n", "\r"] and
+    e = mc.getQualifier() and
     branch = false
-    or
-    ma.getMethod().hasName("matches") and
-    (
-      ma.getMethod().getDeclaringType() instanceof TypeString and
-      e = ma.getQualifier()
-      or
-      ma.getMethod().getDeclaringType().hasQualifiedName("java.util.regex", "Pattern") and
-      e = ma.getArgument(1)
-    ) and
-    (
-      // Allow anything except line breaks
-      (
-        not target.getStringValue().matches("%[^%]%") and
-        not target.getStringValue().matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
-        or
-        target.getStringValue().matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%")
-      ) and
-      branch = true
-      or
-      // Disallow line breaks
-      (
-        not target.getStringValue().matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%") and
-        // Assuming a regex containing line breaks is correctly matching line breaks in a string
-        target.getStringValue().matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
-      ) and
-      branch = false
-    )
+  )
+  or
+  exists(RegexMatch rm, CompileTimeConstantExpr target |
+    rm = g and
+    not rm instanceof Annotation and
+    target = rm.getRegex() and
+    e = rm.getASanitizedExpr()
+  |
+    regexPreventsLogInjection(target.getStringValue(), branch)
   )
 }
+
+/**
+ * Holds if `regex` matches against a pattern that allows anything except
+ * line breaks when `branch` is `true`, or a pattern that matches line breaks
+ * when `branch` is `false`.
+ */
+bindingset[regex]
+private predicate regexPreventsLogInjection(string regex, boolean branch) {
+  // Allow anything except line breaks
+  (
+    not regex.matches("%[^%]%") and
+    not regex.matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
+    or
+    regex.matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%")
+  ) and
+  branch = true
+  or
+  // Disallow line breaks
+  (
+    not regex.matches("%[^%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%]%") and
+    // Assuming a regex containing line breaks is correctly matching line breaks in a string
+    regex.matches("%" + ["\n", "\r", "\\n", "\\r", "\\R"] + "%")
+  ) and
+  branch = false
+}

java/ql/test/query-tests/security/CWE-117/LogInjectionTest.ext.yml

@@ -4,3 +4,5 @@ extensions:
       extensible: sourceModel
     data:
       - ["loginjection", "LogInjectionTest", False, "source", "()", "", "ReturnValue", "remote", "manual"]
+      - ["loginjection", "LogInjectionTest", False, "validatedInput", "()", "", "ReturnValue", "remote", "manual"]
+      - ["loginjection", "LogInjectionTest", False, "validatedInputField", "", "", "", "remote", "manual"]

java/ql/test/query-tests/security/CWE-117/LogInjectionTest.java

@@ -19,6 +19,14 @@ import org.jboss.logging.BasicLogger;
 import org.slf4j.spi.LoggingEventBuilder;
 
 public class LogInjectionTest {
+    @javax.validation.constraints.Pattern(regexp = "^[a-zA-Z0-9]*$")
+    public String validatedInputField;
+
+    @javax.validation.constraints.Pattern(regexp = "[^\n\r]*")
+    public String validatedInput() {
+        return (String) source();
+    }
+
     public Object source() {
         return null;
     }
@@ -187,6 +195,8 @@ public class LogInjectionTest {
             logger.debug(source); // $ MISSING: $ Alert
         }
 
+        logger.debug(validatedInputField);
+        logger.debug(validatedInput());
     }
 
     public void test() {

java/ql/test/query-tests/security/CWE-117/options

@@ -1 +1 @@
-//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/apache-log4j-1.2.17:${testdir}/../../../stubs/apache-log4j-2.14.1:${testdir}/../../../stubs/apache-commons-logging-1.2:${testdir}/../../../stubs/jboss-logging-3.4.2:${testdir}/../../../stubs/slf4j-2.0.0:${testdir}/../../../stubs/scijava-common-2.87.1:${testdir}/../../../stubs/flogger-0.7.1:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/apache-cxf
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/javax-validation-constraints:${testdir}/../../../stubs/apache-log4j-1.2.17:${testdir}/../../../stubs/apache-log4j-2.14.1:${testdir}/../../../stubs/apache-commons-logging-1.2:${testdir}/../../../stubs/jboss-logging-3.4.2:${testdir}/../../../stubs/slf4j-2.0.0:${testdir}/../../../stubs/scijava-common-2.87.1:${testdir}/../../../stubs/flogger-0.7.1:${testdir}/../../../stubs/google-android-9.0.0:${testdir}/../../../stubs/apache-cxf