diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index 25454d80c717..7058b844cbdb 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -6,10 +6,14 @@ import semmle.code.java.dataflow.TaintTracking import semmle.code.java.security.SensitiveActions import semmle.code.java.frameworks.android.Compose private import semmle.code.java.security.Sanitizers +private import semmle.code.java.dataflow.RangeAnalysis /** A data flow source node for sensitive logging sources. */ abstract class SensitiveLoggerSource extends DataFlow::Node { } +/** A data flow barrier node for sensitive logging sanitizers. */ +abstract class SensitiveLoggerBarrier extends DataFlow::Node { } + /** A variable that may hold sensitive information, judging by its name. */ class VariableWithSensitiveName extends Variable { VariableWithSensitiveName() { @@ -40,17 +44,89 @@ private class TypeType extends RefType { } } +/** + * A sanitizer that may remove sensitive information from a string before logging. + * + * It allows for substring operations taking the first N (or last N, for Kotlin) characters, limited to 7 or fewer. + */ +private class PrefixSuffixBarrier extends SensitiveLoggerBarrier { + PrefixSuffixBarrier() { + exists(MethodCall mc, Method m, int limit | + limit = 7 and + mc.getMethod() = m + | + // substring in Java + ( + m.hasQualifiedName("java.lang", "String", "substring") or + m.hasQualifiedName("java.lang", "StringBuffer", "substring") or + m.hasQualifiedName("java.lang", "StringBuilder", "substring") + ) and + ( + twoArgLimit(mc, limit, false) or + singleArgLimit(mc, limit, false) + ) and + this.asExpr() = mc.getQualifier() + or + // Kotlin string operations, which use extension methods (so the string is the first argument) + ( + m.hasQualifiedName("kotlin.text", "StringsKt", "substring") and + ( + twoArgLimit(mc, limit, true) or + singleArgLimit(mc, limit, true) + ) + or + m.hasQualifiedName("kotlin.text", "StringsKt", ["take", "takeLast"]) and + singleArgLimit(mc, limit, true) + ) and + this.asExpr() = mc.getArgument(0) + ) + } +} + +/** A predicate to check single-argument method calls for a constant integer below a set limit. */ +bindingset[limit, isKotlin] +private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { + mc.getNumArgument() = 1 and + exists(int firstArgIndex, int delta | + if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0 + | + bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), delta, true, _) and + delta <= limit + ) +} + +/** A predicate to check two-argument method calls for zero and a constant integer below a set limit. */ +bindingset[limit, isKotlin] +private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { + mc.getNumArgument() = 2 and + 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).(CompileTimeConstantExpr).getIntValue() = 0 and + bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), 0, true, _) and + bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), 0, false, _) and + bounded(mc.getArgument(secondArgIndex), any(ZeroBound z), delta, true, _) and + delta <= limit + ) +} + +private class DefaultSensitiveLoggerBarrier extends SensitiveLoggerBarrier { + DefaultSensitiveLoggerBarrier() { + this.asExpr() instanceof LiveLiteral or + this instanceof SimpleTypeSanitizer or + this.getType() instanceof TypeType + } +} + /** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */ module SensitiveLoggerConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source instanceof SensitiveLoggerSource } predicate isSink(DataFlow::Node sink) { sinkNode(sink, "log-injection") } - predicate isBarrier(DataFlow::Node sanitizer) { - sanitizer.asExpr() instanceof LiveLiteral or - sanitizer instanceof SimpleTypeSanitizer or - sanitizer.getType() instanceof TypeType - } + predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof SensitiveLoggerBarrier } predicate isBarrierIn(DataFlow::Node node) { isSource(node) } diff --git a/java/ql/src/change-notes/2025-11-17-sensitive-logging-new-sanitizers.md b/java/ql/src/change-notes/2025-11-17-sensitive-logging-new-sanitizers.md new file mode 100644 index 000000000000..a3266e4d9e60 --- /dev/null +++ b/java/ql/src/change-notes/2025-11-17-sensitive-logging-new-sanitizers.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Operations that extract only a fixed-length prefix or suffix of a string (for example, `substring` in Java or `take` in Kotlin), when limited to a length of at most 7 characters, are now treated as sanitizers for the `java/sensitive-log` query. \ No newline at end of file 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 e32ff5654e25..54f1e9f8a5aa 100644 --- a/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected +++ b/java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected @@ -1,15 +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: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: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: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 cf983afc287d..6521f7e2df79 100644 --- a/java/ql/test/query-tests/security/CWE-532/Test.java +++ b/java/ql/test/query-tests/security/CWE-532/Test.java @@ -3,11 +3,22 @@ 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 = 4L; logger.info("User's password is: " + password); // $ Alert logger.error("Auth failed for: " + authToken); // $ Alert logger.error("Auth failed for: " + username); // Safe 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 } }