hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2025-12-17 01:03:14 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #20741 from aegilops/java-kotlin-sensitive-logging-substring-barriers

Browse Source 
java: Added Java/Kotlin Sensitive Logging barriers (substrings)
This commit is contained in:
Owen Mansel-Chan
2025-11-26 09:55:14 +00:00
committed by GitHub
parent 0245b9d609 f0dec21b9b
commit 0414555e43
4 changed files with 117 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll
View File

@@ -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) }

4
java/ql/src/change-notes/2025-11-17-sensitive-logging-new-sanitizers.md Normal file
View File

@@ -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.

29
java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected
View File

@@ -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

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

@@ -3,11 +3,22 @@ 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 = 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
}
}