Java: Improve java/log-injection with DEBUG/TRACE exclusion, URLEncoder sanitizer, and test file exclusion

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

@@ -30,12 +30,36 @@ class LogInjectionAdditionalTaintStep extends Unit {
 }
 
 private class DefaultLogInjectionSink extends LogInjectionSink {
-  DefaultLogInjectionSink() { sinkNode(this, "log-injection") }
+  DefaultLogInjectionSink() {
+    sinkNode(this, "log-injection") and
+    not exists(MethodCall mc |
+      this.asExpr() = mc.getAnArgument() and
+      mc.getMethod().getName() in [
+        "debug", "trace", // SLF4J, Log4j, Commons Logging, JBoss Logging
+        "fine", "finer", "finest" // java.util.logging
+      ]
+    )
+  }
 }
 
 private class DefaultLogInjectionSanitizer extends LogInjectionSanitizer instanceof SimpleTypeSanitizer
 { }
 
+/**
+ * A call to `java.net.URLEncoder.encode(...)` is considered a sanitizer for log injection,
+ * since URL encoding replaces line breaks and other special characters with percent-encoded
+ * equivalents that cannot be used to forge log entries.
+ */
+private class UrlEncoderSanitizer extends LogInjectionSanitizer {
+  UrlEncoderSanitizer() {
+    exists(MethodCall mc |
+      mc.getMethod().getDeclaringType().hasQualifiedName("java.net", "URLEncoder") and
+      mc.getMethod().hasName("encode") and
+      this.asExpr() = mc
+    )
+  }
+}
+
 private class LineBreaksLogInjectionSanitizer extends LogInjectionSanitizer {
   LineBreaksLogInjectionSanitizer() {
     logInjectionSanitizer(this.asExpr())

java/ql/src/Security/CWE/CWE-117/LogInjection.ql

@@ -13,9 +13,12 @@
 
 import java
 import semmle.code.java.security.LogInjectionQuery
+import semmle.code.java.dataflow.internal.ModelExclusions
 import LogInjectionFlow::PathGraph
 
 from LogInjectionFlow::PathNode source, LogInjectionFlow::PathNode sink
-where LogInjectionFlow::flowPath(source, sink)
+where
+  LogInjectionFlow::flowPath(source, sink) and
+  not isInTestFile(sink.getNode().getLocation().getFile())
 select sink.getNode(), source, sink, "This log entry depends on a $@.", source.getNode(),
   "user-provided value"

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

@@ -0,0 +1,52 @@
+package loginjection;
+
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * Test cases for URLEncoder.encode() as a sanitizer for log injection,
+ * and for debug/trace level log exclusion.
+ */
+public class LogInjectionUrlEncoderTest {
+
+    public Object source() {
+        return null;
+    }
+
+    // GOOD: URLEncoder.encode() sanitizes the input (percent-encodes line breaks)
+    public void testUrlEncoderSanitizer() {
+        String source = (String) source(); // $ Source
+        Logger logger = null;
+        String encoded = URLEncoder.encode(source, StandardCharsets.UTF_8);
+        logger.warn(encoded); // Safe - URLEncoder.encode sanitizes newlines
+    }
+
+    // GOOD: debug-level logging should not be flagged
+    public void testDebugLevelExcluded() {
+        String source = (String) source(); // $ Source
+        Logger logger = null;
+        logger.debug(source); // Safe - debug level excluded
+    }
+
+    // GOOD: trace-level logging should not be flagged
+    public void testTraceLevelExcluded() {
+        String source = (String) source(); // $ Source
+        Logger logger = null;
+        logger.trace(source); // Safe - trace level excluded
+    }
+
+    // BAD: warn-level logging with unsanitized input should still be flagged
+    public void testWarnLevelFlagged() {
+        String source = (String) source(); // $ Source
+        Logger logger = null;
+        logger.warn(source); // $ Alert
+    }
+
+    // BAD: error-level logging with unsanitized input should still be flagged
+    public void testErrorLevelFlagged() {
+        String source = (String) source(); // $ Source
+        Logger logger = null;
+        logger.error(source); // $ Alert
+    }
+}