Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
86 changes: 81 additions & 5 deletions java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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).getUnderlyingExpr(), 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).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
)
}

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

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
@@ -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 changes: 11 additions & 0 deletions java/ql/test/query-tests/security/CWE-532/Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 = 4;

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