From ec381e4ec579728b9b2ce9cbe99f92b4cc45b414 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Fri, 21 Nov 2025 10:31:50 +0000 Subject: [PATCH] Use range analysis and improve tests --- .../java/security/SensitiveLoggingQuery.qll | 28 +++++++------ .../CWE-532/SensitiveLogInfo.expected | 40 +++++++++---------- .../query-tests/security/CWE-532/Test.java | 7 ++++ 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index b4eecdcdb4c..315f915b2e1 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -87,10 +87,11 @@ private class PrefixSuffixBarrier extends SensitiveLoggerBarrier { bindingset[limit, isKotlin] private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { mc.getNumArgument() = 1 and - exists(int firstArgIndex | - (if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0) and - mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() <= - limit + exists(int firstArgIndex, int delta | + if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0 + | + bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), delta, true, _) and + delta <= limit ) } @@ -98,15 +99,16 @@ private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { bindingset[limit, isKotlin] private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { mc.getNumArgument() = 2 and - exists(int firstArgIndex, int secondArgIndex | - ( - isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2 - or - isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1 - ) and - mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and - mc.getArgument(secondArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() <= - limit + exists(int firstArgIndex, int secondArgIndex, int delta | + isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2 + or + isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1 + | + // mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and + bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), 0, true, _) and + bounded(mc.getArgument(firstArgIndex).getUnderlyingExpr(), any(ZeroBound z), 0, false, _) and + bounded(mc.getArgument(secondArgIndex).getUnderlyingExpr(), any(ZeroBound z), delta, true, _) and + delta <= limit ) } diff --git a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected index 3827d2894ad..54f1e9f8a5a 100644 --- a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected +++ b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected @@ -1,28 +1,28 @@ #select -| Test.java:7:21:7:53 | ... + ... | Test.java:7:46:7:53 | password : String | Test.java:7:21:7:53 | ... + ... | This $@ is written to a log file. | Test.java:7:46:7:53 | password | potentially sensitive information | -| Test.java:8:22:8:52 | ... + ... | Test.java:8:44:8:52 | authToken : String | Test.java:8:22:8:52 | ... + ... | This $@ is written to a log file. | Test.java:8:44:8:52 | authToken | potentially sensitive information | -| Test.java:14:22:14:75 | ... + ... | Test.java:14:44:14:52 | authToken : String | Test.java:14:22:14:75 | ... + ... | This $@ is written to a log file. | Test.java:14:44:14:52 | authToken | potentially sensitive information | -| Test.java:15:22:15:75 | ... + ... | Test.java:15:44:15:52 | authToken : String | Test.java:15:22:15:75 | ... + ... | This $@ is written to a log file. | Test.java:15:44:15:52 | authToken | potentially sensitive information | +| Test.java:11:21:11:53 | ... + ... | Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | This $@ is written to a log file. | Test.java:11:46:11:53 | password | potentially sensitive information | +| Test.java:12:22:12:52 | ... + ... | Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | This $@ is written to a log file. | Test.java:12:44:12:52 | authToken | potentially sensitive information | +| Test.java:21:22:21:75 | ... + ... | Test.java:21:44:21:52 | authToken : String | Test.java:21:22:21:75 | ... + ... | This $@ is written to a log file. | Test.java:21:44:21:52 | authToken | potentially sensitive information | +| Test.java:22:22:22:75 | ... + ... | Test.java:22:44:22:52 | authToken : String | Test.java:22:22:22:75 | ... + ... | This $@ is written to a log file. | Test.java:22:44:22:52 | authToken | potentially sensitive information | edges -| Test.java:7:46:7:53 | password : String | Test.java:7:21:7:53 | ... + ... | provenance | Sink:MaD:2 | -| Test.java:8:44:8:52 | authToken : String | Test.java:8:22:8:52 | ... + ... | provenance | Sink:MaD:1 | -| Test.java:14:44:14:52 | authToken : String | Test.java:14:44:14:67 | substring(...) : String | provenance | MaD:3 | -| Test.java:14:44:14:67 | substring(...) : String | Test.java:14:22:14:75 | ... + ... | provenance | Sink:MaD:1 | -| Test.java:15:44:15:52 | authToken : String | Test.java:15:44:15:67 | substring(...) : String | provenance | MaD:3 | -| Test.java:15:44:15:67 | substring(...) : String | Test.java:15:22:15:75 | ... + ... | provenance | Sink:MaD:1 | +| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 | +| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 | +| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 | +| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 | +| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 | +| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 | models | 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual | | 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual | | 3 | Summary: java.lang; String; false; substring; ; ; Argument[this]; ReturnValue; taint; manual | nodes -| Test.java:7:21:7:53 | ... + ... | semmle.label | ... + ... | -| Test.java:7:46:7:53 | password : String | semmle.label | password : String | -| Test.java:8:22:8:52 | ... + ... | semmle.label | ... + ... | -| Test.java:8:44:8:52 | authToken : String | semmle.label | authToken : String | -| Test.java:14:22:14:75 | ... + ... | semmle.label | ... + ... | -| Test.java:14:44:14:52 | authToken : String | semmle.label | authToken : String | -| Test.java:14:44:14:67 | substring(...) : String | semmle.label | substring(...) : String | -| Test.java:15:22:15:75 | ... + ... | semmle.label | ... + ... | -| Test.java:15:44:15:52 | authToken : String | semmle.label | authToken : String | -| Test.java:15:44:15:67 | substring(...) : String | semmle.label | substring(...) : String | +| Test.java:11:21:11:53 | ... + ... | semmle.label | ... + ... | +| Test.java:11:46:11:53 | password : String | semmle.label | password : String | +| Test.java:12:22:12:52 | ... + ... | semmle.label | ... + ... | +| Test.java:12:44:12:52 | authToken : String | semmle.label | authToken : String | +| Test.java:21:22:21:75 | ... + ... | semmle.label | ... + ... | +| Test.java:21:44:21:52 | authToken : String | semmle.label | authToken : String | +| Test.java:21:44:21:67 | substring(...) : String | semmle.label | substring(...) : String | +| Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... | +| Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String | +| Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String | subpaths diff --git a/java/ql/test/query-tests/security/CWE-532/Test.java b/java/ql/test/query-tests/security/CWE-532/Test.java index 2eed3e79342..0383f521ff3 100644 --- a/java/ql/test/query-tests/security/CWE-532/Test.java +++ b/java/ql/test/query-tests/security/CWE-532/Test.java @@ -3,6 +3,10 @@ import org.apache.logging.log4j.Logger; class Test { void test(String password, String authToken, String username, String nullToken, String stringTokenizer) { Logger logger = null; + int zero = 0; + int four = 4; + short zeroS = 0; + long fourL = 4; logger.info("User's password is: " + password); // $ Alert logger.error("Auth failed for: " + authToken); // $ Alert @@ -10,7 +14,10 @@ class Test { logger.error("Auth failed for: " + nullToken); // Safe logger.error("Auth failed for: " + stringTokenizer); // Safe logger.error("Auth failed for: " + authToken.substring(4) + "..."); // Safe + logger.error("Auth failed for: " + authToken.substring(four) + "..."); // Safe logger.error("Auth failed for: " + authToken.substring(0,4) + "..."); // Safe + logger.error("Auth failed for: " + authToken.substring(zero,four) + "..."); // Safe + logger.error("Auth failed for: " + authToken.substring((int)zeroS,(int)fourL) + "..."); // Safe logger.error("Auth failed for: " + authToken.substring(1,5) + "..."); // $ Alert logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert }