Merge pull request #17100 from owen-mc/java/sensitive-log/ignore-tokenizer

Java: whitelist variable names containing "tokenizer" for `java/sensitive-log`

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

@@ -28,13 +28,26 @@ private string nonSuspicious() {
 }
 
 /**
- * Gets a regular expression for matching common names of variables that indicate the value being held contains sensitive information.
+ * Gets a regular expression for matching common names of variables that
+ * indicate the value being held contains sensitive information.
  */
 string getCommonSensitiveInfoRegex() {
   result = "(?i).*(challenge|pass(wd|word|code|phrase))(?!.*question).*" or
   result = "(?i).*(token|secret).*"
 }
 
+/**
+ * Gets a regular expression for matching common names of variables that
+ * indicate the value being held does not contains sensitive information,
+ * but is a false positive for `getCommonSensitiveInfoRegex`.
+ *
+ * - "tokenizer" is often used for java.util.StringTokenizer.
+ * - "tokenImage" appears in parser code generated by JavaCC.
+ */
+string getCommonSensitiveInfoFPRegex() {
+  result = "(?i).*(null|tokenizer).*" or result = "tokenImage"
+}
+
 /** An expression that might contain sensitive data. */
 abstract class SensitiveExpr extends Expr { }
 

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

@@ -15,8 +15,7 @@ class VariableWithSensitiveName extends Variable {
   VariableWithSensitiveName() {
     exists(string name | name = this.getName() |
       name.regexpMatch(getCommonSensitiveInfoRegex()) and
-      not name.regexpMatch("(?i).*null.*") and
-      name != "tokenImage" // appears in parser code generated by JavaCC
+      not name.regexpMatch(getCommonSensitiveInfoFPRegex())
     )
   }
 }

java/ql/src/change-notes/2024-07-30-sensitive-log-whitelist-tokenizer.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Variables names containing the string "tokenizer" (case-insensitively) are no longer sources for the `java/sensitive-log` query. They normally relate to things like `java.util.StringTokenizer`, which are not sensitive information. This should fix some false positive alerts.

java/ql/test/query-tests/security/CWE-532/Test.java

@@ -1,28 +1,13 @@
 import org.apache.logging.log4j.Logger;
 
 class Test {
-    void test(String password) {
+    void test(String password, String authToken, String username, String nullToken, String stringTokenizer) {
         Logger logger = null;
 
         logger.info("User's password is: " + password); // $ hasTaintFlow
-    }
-
-    void test2(String authToken) {
-        Logger logger = null;
-
         logger.error("Auth failed for: " + authToken); // $ hasTaintFlow
-    }
-
-    void test3(String username) {
-        Logger logger = null;
-
         logger.error("Auth failed for: " + username); // Safe
-    }
-
-    void test4(String nullToken) {
-        Logger logger = null;
-
         logger.error("Auth failed for: " + nullToken); // Safe
+        logger.error("Auth failed for: " + stringTokenizer); // Safe
     }
-
 }