From 26087f6060e5ccd0f1cb7253c361de3dfee53314 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 31 Oct 2025 17:56:04 +0000 Subject: [PATCH 1/2] Added java-kotlin Sensitive Logging barriers (substrings) --- .../java/security/SensitiveLoggingQuery.qll | 112 +++++++++++++++++- 1 file changed, 107 insertions(+), 5 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index 25454d80c717..855d05168cc6 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 +import semmle.code.java.Constants /** 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,114 @@ 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 SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier { + SensitiveLoggerSanitizerCalled() { + exists(MethodCall mc, Method m, int limit | + limit = 7 and + mc.getMethod() = m and + ( + // 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) 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 + m.hasQualifiedName("kotlin.text", "StringsKt", ["take", "takeLast"]) and + singleArgLimit(mc, limit, true) + ) and + this.asExpr() = mc.getArgument(0) + ) + ) + } +} + +bindingset[limit, isKotlin] +predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { + exists(int argIndex, int staticInt | + (if isKotlin = true then argIndex = 1 else argIndex = 0) and + ( + staticInt <= limit and + staticInt > 0 and + mc.getArgument(argIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = staticInt + or exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | + source.asExpr() = cte and + cte.getIntValue() = staticInt and + sink.asExpr() = mc.getArgument(argIndex) and + IntegerToArgFlow::flow(source, sink) + ) + ) + ) +} + +bindingset[limit, isKotlin] +predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { + exists(int firstArgIndex, int secondArgIndex, int staticInt | + staticInt <= limit and + staticInt > 0 and + ( + (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() = staticInt + or exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | + source.asExpr() = cte and + cte.getIntValue() = staticInt and + sink.asExpr() = mc.getArgument(secondArgIndex) and + IntegerToArgFlow::flow(source, sink) + ) + ) + ) +} + +module IntegerToArgConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { + source.asExpr().getUnderlyingExpr() instanceof CompileTimeConstantExpr and + source.asExpr().getType() instanceof IntegralType and + source.asExpr().(CompileTimeConstantExpr).getIntValue() > 0 + } + + predicate isSink(DataFlow::Node sink) { + exists(MethodCall mc | + sink.asExpr() = mc.getAnArgument() + and sink.asExpr().getType() instanceof IntegralType + ) + } + + predicate isBarrier(DataFlow::Node sanitizer) { none() } + + predicate isBarrierIn(DataFlow::Node node) { none() } +} + +private class GenericSanitizer extends SensitiveLoggerBarrier { + GenericSanitizer() { + 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) } @@ -58,3 +159,4 @@ module SensitiveLoggerConfig implements DataFlow::ConfigSig { } module SensitiveLoggerFlow = TaintTracking::Global; +module IntegerToArgFlow = TaintTracking::Global; From d1eceee9d44463602e6d1167fa6ac3cf98b67572 Mon Sep 17 00:00:00 2001 From: aegilops <41705651+aegilops@users.noreply.github.com> Date: Fri, 31 Oct 2025 18:19:27 +0000 Subject: [PATCH 2/2] Fixed format/docs issues --- .../java/security/SensitiveLoggingQuery.qll | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll index 855d05168cc6..4692427f1cde 100644 --- a/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll +++ b/java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll @@ -44,8 +44,9 @@ private class TypeType extends RefType { } } -/** A sanitizer that may remove sensitive information from a string before logging. - * +/** + * 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 SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier { @@ -65,7 +66,8 @@ private class SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier { 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) + m.hasQualifiedName("kotlin.text", "StringsKt", "substring") and + twoArgLimit(mc, limit, true) or m.hasQualifiedName("kotlin.text", "StringsKt", ["take", "takeLast"]) and singleArgLimit(mc, limit, true) @@ -76,15 +78,18 @@ private class SensitiveLoggerSanitizerCalled extends SensitiveLoggerBarrier { } } +/** A predicate to check single-argument method calls for a constant integer below a set limit. */ bindingset[limit, isKotlin] -predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { +private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { exists(int argIndex, int staticInt | (if isKotlin = true then argIndex = 1 else argIndex = 0) and ( staticInt <= limit and staticInt > 0 and - mc.getArgument(argIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = staticInt - or exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | + mc.getArgument(argIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = + staticInt + or + exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | source.asExpr() = cte and cte.getIntValue() = staticInt and sink.asExpr() = mc.getArgument(argIndex) and @@ -94,21 +99,23 @@ predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) { ) } +/** A predicate to check two-argument method calls for zero and a constant integer below a set limit. */ bindingset[limit, isKotlin] -predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { +private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { exists(int firstArgIndex, int secondArgIndex, int staticInt | staticInt <= limit and staticInt > 0 and ( - (isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2) + isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2 or - (isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1) - ) - and + isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1 + ) and mc.getArgument(firstArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = 0 and ( - mc.getArgument(secondArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = staticInt - or exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | + mc.getArgument(secondArgIndex).getUnderlyingExpr().(CompileTimeConstantExpr).getIntValue() = + staticInt + or + exists(CompileTimeConstantExpr cte, DataFlow::Node source, DataFlow::Node sink | source.asExpr() = cte and cte.getIntValue() = staticInt and sink.asExpr() = mc.getArgument(secondArgIndex) and @@ -118,7 +125,8 @@ predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) { ) } -module IntegerToArgConfig implements DataFlow::ConfigSig { +/** A data-flow configuration for identifying flow from a constant integer to a use in a method argument. */ +private module IntegerToArgConfig implements DataFlow::ConfigSig { predicate isSource(DataFlow::Node source) { source.asExpr().getUnderlyingExpr() instanceof CompileTimeConstantExpr and source.asExpr().getType() instanceof IntegralType and @@ -127,8 +135,8 @@ module IntegerToArgConfig implements DataFlow::ConfigSig { predicate isSink(DataFlow::Node sink) { exists(MethodCall mc | - sink.asExpr() = mc.getAnArgument() - and sink.asExpr().getType() instanceof IntegralType + sink.asExpr() = mc.getAnArgument() and + sink.asExpr().getType() instanceof IntegralType ) } @@ -159,4 +167,5 @@ module SensitiveLoggerConfig implements DataFlow::ConfigSig { } module SensitiveLoggerFlow = TaintTracking::Global; + module IntegerToArgFlow = TaintTracking::Global;