Skip to content

Commit 0414555

Browse files
owen-mcowen-mc
authored
Merge pull request #20741 from aegilops/java-kotlin-sensitive-logging-substring-barriers
java: Added Java/Kotlin Sensitive Logging barriers (substrings)
2 parents 0245b9d + f0dec21 commit 0414555

File tree

4 files changed

+117
-13
lines changed

4 files changed

+117
-13
lines changed

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

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,14 @@ import semmle.code.java.dataflow.TaintTracking
66
import semmle.code.java.security.SensitiveActions
77
import semmle.code.java.frameworks.android.Compose
88
private import semmle.code.java.security.Sanitizers
9+
private import semmle.code.java.dataflow.RangeAnalysis
910

1011
/** A data flow source node for sensitive logging sources. */
1112
abstract class SensitiveLoggerSource extends DataFlow::Node { }
1213

14+
/** A data flow barrier node for sensitive logging sanitizers. */
15+
abstract class SensitiveLoggerBarrier extends DataFlow::Node { }
16+
1317
/** A variable that may hold sensitive information, judging by its name. */
1418
class VariableWithSensitiveName extends Variable {
1519
VariableWithSensitiveName() {
@@ -40,17 +44,89 @@ private class TypeType extends RefType {
4044
}
4145
}
4246

47+
/**
48+
* A sanitizer that may remove sensitive information from a string before logging.
49+
*
50+
* It allows for substring operations taking the first N (or last N, for Kotlin) characters, limited to 7 or fewer.
51+
*/
52+
private class PrefixSuffixBarrier extends SensitiveLoggerBarrier {
53+
PrefixSuffixBarrier() {
54+
exists(MethodCall mc, Method m, int limit |
55+
limit = 7 and
56+
mc.getMethod() = m
57+
|
58+
// substring in Java
59+
(
60+
m.hasQualifiedName("java.lang", "String", "substring") or
61+
m.hasQualifiedName("java.lang", "StringBuffer", "substring") or
62+
m.hasQualifiedName("java.lang", "StringBuilder", "substring")
63+
) and
64+
(
65+
twoArgLimit(mc, limit, false) or
66+
singleArgLimit(mc, limit, false)
67+
) and
68+
this.asExpr() = mc.getQualifier()
69+
or
70+
// Kotlin string operations, which use extension methods (so the string is the first argument)
71+
(
72+
m.hasQualifiedName("kotlin.text", "StringsKt", "substring") and
73+
(
74+
twoArgLimit(mc, limit, true) or
75+
singleArgLimit(mc, limit, true)
76+
)
77+
or
78+
m.hasQualifiedName("kotlin.text", "StringsKt", ["take", "takeLast"]) and
79+
singleArgLimit(mc, limit, true)
80+
) and
81+
this.asExpr() = mc.getArgument(0)
82+
)
83+
}
84+
}
85+
86+
/** A predicate to check single-argument method calls for a constant integer below a set limit. */
87+
bindingset[limit, isKotlin]
88+
private predicate singleArgLimit(MethodCall mc, int limit, boolean isKotlin) {
89+
mc.getNumArgument() = 1 and
90+
exists(int firstArgIndex, int delta |
91+
if isKotlin = true then firstArgIndex = 1 else firstArgIndex = 0
92+
|
93+
bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), delta, true, _) and
94+
delta <= limit
95+
)
96+
}
97+
98+
/** A predicate to check two-argument method calls for zero and a constant integer below a set limit. */
99+
bindingset[limit, isKotlin]
100+
private predicate twoArgLimit(MethodCall mc, int limit, boolean isKotlin) {
101+
mc.getNumArgument() = 2 and
102+
exists(int firstArgIndex, int secondArgIndex, int delta |
103+
isKotlin = true and firstArgIndex = 1 and secondArgIndex = 2
104+
or
105+
isKotlin = false and firstArgIndex = 0 and secondArgIndex = 1
106+
|
107+
// mc.getArgument(firstArgIndex).(CompileTimeConstantExpr).getIntValue() = 0 and
108+
bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), 0, true, _) and
109+
bounded(mc.getArgument(firstArgIndex), any(ZeroBound z), 0, false, _) and
110+
bounded(mc.getArgument(secondArgIndex), any(ZeroBound z), delta, true, _) and
111+
delta <= limit
112+
)
113+
}
114+
115+
private class DefaultSensitiveLoggerBarrier extends SensitiveLoggerBarrier {
116+
DefaultSensitiveLoggerBarrier() {
117+
this.asExpr() instanceof LiveLiteral or
118+
this instanceof SimpleTypeSanitizer or
119+
this.getType() instanceof TypeType
120+
}
121+
}
122+
43123
/** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */
44124
module SensitiveLoggerConfig implements DataFlow::ConfigSig {
45125
predicate isSource(DataFlow::Node source) { source instanceof SensitiveLoggerSource }
46126

47127
predicate isSink(DataFlow::Node sink) { sinkNode(sink, "log-injection") }
48128

49-
predicate isBarrier(DataFlow::Node sanitizer) {
50-
sanitizer.asExpr() instanceof LiveLiteral or
51-
sanitizer instanceof SimpleTypeSanitizer or
52-
sanitizer.getType() instanceof TypeType
53-
}
129+
predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof SensitiveLoggerBarrier }
54130

55131
predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
56132

‎java/ql/src/change-notes/2025-11-17-sensitive-logging-new-sanitizers.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* 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.

‎java/ql/test/query-tests/security/CWE-532/SensitiveLogInfo.expected‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,28 @@
11
#select
2-
| 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 |
3-
| 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 |
2+
| 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 |
3+
| 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 |
4+
| 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 |
5+
| 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 |
46
edges
5-
| Test.java:7:46:7:53 | password : String | Test.java:7:21:7:53 | ... + ... | provenance | Sink:MaD:2 |
6-
| Test.java:8:44:8:52 | authToken : String | Test.java:8:22:8:52 | ... + ... | provenance | Sink:MaD:1 |
7+
| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 |
8+
| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 |
9+
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 |
10+
| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 |
11+
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 |
12+
| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 |
713
models
814
| 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual |
915
| 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual |
16+
| 3 | Summary: java.lang; String; false; substring; ; ; Argument[this]; ReturnValue; taint; manual |
1017
nodes
11-
| Test.java:7:21:7:53 | ... + ... | semmle.label | ... + ... |
12-
| Test.java:7:46:7:53 | password : String | semmle.label | password : String |
13-
| Test.java:8:22:8:52 | ... + ... | semmle.label | ... + ... |
14-
| Test.java:8:44:8:52 | authToken : String | semmle.label | authToken : String |
18+
| Test.java:11:21:11:53 | ... + ... | semmle.label | ... + ... |
19+
| Test.java:11:46:11:53 | password : String | semmle.label | password : String |
20+
| Test.java:12:22:12:52 | ... + ... | semmle.label | ... + ... |
21+
| Test.java:12:44:12:52 | authToken : String | semmle.label | authToken : String |
22+
| Test.java:21:22:21:75 | ... + ... | semmle.label | ... + ... |
23+
| Test.java:21:44:21:52 | authToken : String | semmle.label | authToken : String |
24+
| Test.java:21:44:21:67 | substring(...) : String | semmle.label | substring(...) : String |
25+
| Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... |
26+
| Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String |
27+
| Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String |
1528
subpaths

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,22 @@
33
class Test {
44
void test(String password, String authToken, String username, String nullToken, String stringTokenizer) {
55
Logger logger = null;
6+
int zero = 0;
7+
int four = 4;
8+
short zeroS = 0;
9+
long fourL = 4L;
610

711
logger.info("User's password is: " + password); // $ Alert
812
logger.error("Auth failed for: " + authToken); // $ Alert
913
logger.error("Auth failed for: " + username); // Safe
1014
logger.error("Auth failed for: " + nullToken); // Safe
1115
logger.error("Auth failed for: " + stringTokenizer); // Safe
16+
logger.error("Auth failed for: " + authToken.substring(4) + "..."); // Safe
17+
logger.error("Auth failed for: " + authToken.substring(four) + "..."); // Safe
18+
logger.error("Auth failed for: " + authToken.substring(0,4) + "..."); // Safe
19+
logger.error("Auth failed for: " + authToken.substring(zero,four) + "..."); // Safe
20+
logger.error("Auth failed for: " + authToken.substring((int)zeroS,(int)fourL) + "..."); // Safe
21+
logger.error("Auth failed for: " + authToken.substring(1,5) + "..."); // $ Alert
22+
logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert
1223
}
1324
}

0 commit comments

Comments
 (0)