Skip to content

Commit 87c35e6

Browse files
authored
Merge pull request #21654 from MarkLee131/fix/sensitive-log-hash-sanitizer
Java: treat hash/encrypt/digest methods as sensitive-log sanitizers
2 parents a473fdb + 28a6ff2 commit 87c35e6

6 files changed

Lines changed: 73 additions & 43 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `java/sensitive-log` query now treats method calls whose names contain "encrypt", "hash", or "digest" as sanitizers, consistent with the existing treatment in `java/cleartext-storage-in-log`. This reduces false positives when sensitive data is hashed or encrypted before logging.

java/ql/lib/semmle/code/java/security/CleartextStorageQuery.qll

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import java
44
private import semmle.code.java.dataflow.TaintTracking
5+
private import semmle.code.java.security.Sanitizers
56
private import semmle.code.java.security.SensitiveActions
67

78
/** A sink representing persistent storage that saves data in clear text. */
@@ -76,17 +77,6 @@ private class DefaultCleartextStorageSanitizer extends CleartextStorageSanitizer
7677
}
7778
}
7879

79-
/**
80-
* Method call for encrypting sensitive information. As there are various implementations of
81-
* encryption (reversible and non-reversible) from both JDK and third parties, this class simply
82-
* checks method name to take a best guess to reduce false positives.
83-
*/
84-
private class EncryptedSensitiveMethodCall extends MethodCall {
85-
EncryptedSensitiveMethodCall() {
86-
this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"])
87-
}
88-
}
89-
9080
/** Flow configuration for encryption methods flowing to inputs of persistent storage. */
9181
private module EncryptedValueFlowConfig implements DataFlow::ConfigSig {
9282
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof EncryptedSensitiveMethodCall }

java/ql/lib/semmle/code/java/security/Sanitizers.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,14 @@ class RegexpCheckBarrier extends DataFlow::Node {
6363
exists(RegexMatch rm | rm instanceof Annotation | this.asExpr() = rm.getString())
6464
}
6565
}
66+
67+
/**
68+
* A method call for encrypting, hashing, or digesting sensitive information. As there are various
69+
* implementations of encryption (reversible and non-reversible) from both JDK and third parties,
70+
* this class simply checks the method name to take a best guess to reduce false positives.
71+
*/
72+
class EncryptedSensitiveMethodCall extends MethodCall {
73+
EncryptedSensitiveMethodCall() {
74+
this.getMethod().getName().toLowerCase().matches(["%encrypt%", "%hash%", "%digest%"])
75+
}
76+
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,14 @@ private class DefaultSensitiveLoggerBarrier extends SensitiveLoggerBarrier {
120120
}
121121
}
122122

123+
/**
124+
* A barrier for sensitive data that has been hashed, encrypted, or digested before logging.
125+
* This is consistent with the treatment of encryption in `CleartextStorageQuery.qll` (CWE-312).
126+
*/
127+
private class EncryptionBarrier extends SensitiveLoggerBarrier {
128+
EncryptionBarrier() { this.asExpr() instanceof EncryptedSensitiveMethodCall }
129+
}
130+
123131
/** A data-flow configuration for identifying potentially-sensitive data flowing to a log output. */
124132
module SensitiveLoggerConfig implements DataFlow::ConfigSig {
125133
predicate isSource(DataFlow::Node source) { source instanceof SensitiveLoggerSource }

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

Lines changed: 36 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,31 @@
33
| 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 |
44
| 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 |
55
| 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 |
6-
| Test.java:66:21:66:43 | ... + ... | Test.java:66:33:66:43 | accessToken : String | Test.java:66:21:66:43 | ... + ... | This $@ is written to a log file. | Test.java:66:33:66:43 | accessToken | potentially sensitive information |
7-
| Test.java:67:21:67:45 | ... + ... | Test.java:67:34:67:45 | clientSecret : String | Test.java:67:21:67:45 | ... + ... | This $@ is written to a log file. | Test.java:67:34:67:45 | clientSecret | potentially sensitive information |
8-
| Test.java:68:21:68:42 | ... + ... | Test.java:68:34:68:42 | apiSecret : String | Test.java:68:21:68:42 | ... + ... | This $@ is written to a log file. | Test.java:68:34:68:42 | apiSecret | potentially sensitive information |
9-
| Test.java:69:21:69:44 | ... + ... | Test.java:69:33:69:44 | sessionToken : String | Test.java:69:21:69:44 | ... + ... | This $@ is written to a log file. | Test.java:69:33:69:44 | sessionToken | potentially sensitive information |
10-
| Test.java:70:21:70:43 | ... + ... | Test.java:70:33:70:43 | bearerToken : String | Test.java:70:21:70:43 | ... + ... | This $@ is written to a log file. | Test.java:70:33:70:43 | bearerToken | potentially sensitive information |
11-
| Test.java:71:21:71:39 | ... + ... | Test.java:71:31:71:39 | secretKey : String | Test.java:71:21:71:39 | ... + ... | This $@ is written to a log file. | Test.java:71:31:71:39 | secretKey | potentially sensitive information |
12-
| Test.java:72:21:72:44 | ... + ... | Test.java:72:33:72:44 | refreshToken : String | Test.java:72:21:72:44 | ... + ... | This $@ is written to a log file. | Test.java:72:33:72:44 | refreshToken | potentially sensitive information |
13-
| Test.java:73:21:73:43 | ... + ... | Test.java:73:33:73:43 | secretValue : String | Test.java:73:21:73:43 | ... + ... | This $@ is written to a log file. | Test.java:73:33:73:43 | secretValue | potentially sensitive information |
6+
| Test.java:31:21:31:37 | ... + ... | Test.java:31:30:31:37 | password : String | Test.java:31:21:31:37 | ... + ... | This $@ is written to a log file. | Test.java:31:30:31:37 | password | potentially sensitive information |
7+
| Test.java:75:21:75:43 | ... + ... | Test.java:75:33:75:43 | accessToken : String | Test.java:75:21:75:43 | ... + ... | This $@ is written to a log file. | Test.java:75:33:75:43 | accessToken | potentially sensitive information |
8+
| Test.java:76:21:76:45 | ... + ... | Test.java:76:34:76:45 | clientSecret : String | Test.java:76:21:76:45 | ... + ... | This $@ is written to a log file. | Test.java:76:34:76:45 | clientSecret | potentially sensitive information |
9+
| Test.java:77:21:77:42 | ... + ... | Test.java:77:34:77:42 | apiSecret : String | Test.java:77:21:77:42 | ... + ... | This $@ is written to a log file. | Test.java:77:34:77:42 | apiSecret | potentially sensitive information |
10+
| Test.java:78:21:78:44 | ... + ... | Test.java:78:33:78:44 | sessionToken : String | Test.java:78:21:78:44 | ... + ... | This $@ is written to a log file. | Test.java:78:33:78:44 | sessionToken | potentially sensitive information |
11+
| Test.java:79:21:79:43 | ... + ... | Test.java:79:33:79:43 | bearerToken : String | Test.java:79:21:79:43 | ... + ... | This $@ is written to a log file. | Test.java:79:33:79:43 | bearerToken | potentially sensitive information |
12+
| Test.java:80:21:80:39 | ... + ... | Test.java:80:31:80:39 | secretKey : String | Test.java:80:21:80:39 | ... + ... | This $@ is written to a log file. | Test.java:80:31:80:39 | secretKey | potentially sensitive information |
13+
| Test.java:81:21:81:44 | ... + ... | Test.java:81:33:81:44 | refreshToken : String | Test.java:81:21:81:44 | ... + ... | This $@ is written to a log file. | Test.java:81:33:81:44 | refreshToken | potentially sensitive information |
14+
| Test.java:82:21:82:43 | ... + ... | Test.java:82:33:82:43 | secretValue : String | Test.java:82:21:82:43 | ... + ... | This $@ is written to a log file. | Test.java:82:33:82:43 | secretValue | potentially sensitive information |
1415
edges
1516
| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 |
1617
| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 |
1718
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 |
1819
| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 |
1920
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 |
2021
| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 |
21-
| Test.java:66:33:66:43 | accessToken : String | Test.java:66:21:66:43 | ... + ... | provenance | Sink:MaD:2 |
22-
| Test.java:67:34:67:45 | clientSecret : String | Test.java:67:21:67:45 | ... + ... | provenance | Sink:MaD:2 |
23-
| Test.java:68:34:68:42 | apiSecret : String | Test.java:68:21:68:42 | ... + ... | provenance | Sink:MaD:2 |
24-
| Test.java:69:33:69:44 | sessionToken : String | Test.java:69:21:69:44 | ... + ... | provenance | Sink:MaD:2 |
25-
| Test.java:70:33:70:43 | bearerToken : String | Test.java:70:21:70:43 | ... + ... | provenance | Sink:MaD:2 |
26-
| Test.java:71:31:71:39 | secretKey : String | Test.java:71:21:71:39 | ... + ... | provenance | Sink:MaD:2 |
27-
| Test.java:72:33:72:44 | refreshToken : String | Test.java:72:21:72:44 | ... + ... | provenance | Sink:MaD:2 |
28-
| Test.java:73:33:73:43 | secretValue : String | Test.java:73:21:73:43 | ... + ... | provenance | Sink:MaD:2 |
22+
| Test.java:31:30:31:37 | password : String | Test.java:31:21:31:37 | ... + ... | provenance | Sink:MaD:2 |
23+
| Test.java:75:33:75:43 | accessToken : String | Test.java:75:21:75:43 | ... + ... | provenance | Sink:MaD:2 |
24+
| Test.java:76:34:76:45 | clientSecret : String | Test.java:76:21:76:45 | ... + ... | provenance | Sink:MaD:2 |
25+
| Test.java:77:34:77:42 | apiSecret : String | Test.java:77:21:77:42 | ... + ... | provenance | Sink:MaD:2 |
26+
| Test.java:78:33:78:44 | sessionToken : String | Test.java:78:21:78:44 | ... + ... | provenance | Sink:MaD:2 |
27+
| Test.java:79:33:79:43 | bearerToken : String | Test.java:79:21:79:43 | ... + ... | provenance | Sink:MaD:2 |
28+
| Test.java:80:31:80:39 | secretKey : String | Test.java:80:21:80:39 | ... + ... | provenance | Sink:MaD:2 |
29+
| Test.java:81:33:81:44 | refreshToken : String | Test.java:81:21:81:44 | ... + ... | provenance | Sink:MaD:2 |
30+
| Test.java:82:33:82:43 | secretValue : String | Test.java:82:21:82:43 | ... + ... | provenance | Sink:MaD:2 |
2931
models
3032
| 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual |
3133
| 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual |
@@ -41,20 +43,22 @@ nodes
4143
| Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... |
4244
| Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String |
4345
| Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String |
44-
| Test.java:66:21:66:43 | ... + ... | semmle.label | ... + ... |
45-
| Test.java:66:33:66:43 | accessToken : String | semmle.label | accessToken : String |
46-
| Test.java:67:21:67:45 | ... + ... | semmle.label | ... + ... |
47-
| Test.java:67:34:67:45 | clientSecret : String | semmle.label | clientSecret : String |
48-
| Test.java:68:21:68:42 | ... + ... | semmle.label | ... + ... |
49-
| Test.java:68:34:68:42 | apiSecret : String | semmle.label | apiSecret : String |
50-
| Test.java:69:21:69:44 | ... + ... | semmle.label | ... + ... |
51-
| Test.java:69:33:69:44 | sessionToken : String | semmle.label | sessionToken : String |
52-
| Test.java:70:21:70:43 | ... + ... | semmle.label | ... + ... |
53-
| Test.java:70:33:70:43 | bearerToken : String | semmle.label | bearerToken : String |
54-
| Test.java:71:21:71:39 | ... + ... | semmle.label | ... + ... |
55-
| Test.java:71:31:71:39 | secretKey : String | semmle.label | secretKey : String |
56-
| Test.java:72:21:72:44 | ... + ... | semmle.label | ... + ... |
57-
| Test.java:72:33:72:44 | refreshToken : String | semmle.label | refreshToken : String |
58-
| Test.java:73:21:73:43 | ... + ... | semmle.label | ... + ... |
59-
| Test.java:73:33:73:43 | secretValue : String | semmle.label | secretValue : String |
46+
| Test.java:31:21:31:37 | ... + ... | semmle.label | ... + ... |
47+
| Test.java:31:30:31:37 | password : String | semmle.label | password : String |
48+
| Test.java:75:21:75:43 | ... + ... | semmle.label | ... + ... |
49+
| Test.java:75:33:75:43 | accessToken : String | semmle.label | accessToken : String |
50+
| Test.java:76:21:76:45 | ... + ... | semmle.label | ... + ... |
51+
| Test.java:76:34:76:45 | clientSecret : String | semmle.label | clientSecret : String |
52+
| Test.java:77:21:77:42 | ... + ... | semmle.label | ... + ... |
53+
| Test.java:77:34:77:42 | apiSecret : String | semmle.label | apiSecret : String |
54+
| Test.java:78:21:78:44 | ... + ... | semmle.label | ... + ... |
55+
| Test.java:78:33:78:44 | sessionToken : String | semmle.label | sessionToken : String |
56+
| Test.java:79:21:79:43 | ... + ... | semmle.label | ... + ... |
57+
| Test.java:79:33:79:43 | bearerToken : String | semmle.label | bearerToken : String |
58+
| Test.java:80:21:80:39 | ... + ... | semmle.label | ... + ... |
59+
| Test.java:80:31:80:39 | secretKey : String | semmle.label | secretKey : String |
60+
| Test.java:81:21:81:44 | ... + ... | semmle.label | ... + ... |
61+
| Test.java:81:33:81:44 | refreshToken : String | semmle.label | refreshToken : String |
62+
| Test.java:82:21:82:43 | ... + ... | semmle.label | ... + ... |
63+
| Test.java:82:33:82:43 | secretValue : String | semmle.label | secretValue : String |
6064
subpaths

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,15 @@ void test(String password, String authToken, String username, String nullToken,
2222
logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert
2323
}
2424

25+
// Tests for hash/encryption sanitizer
26+
void testHashSanitizer(String password, String authToken) {
27+
Logger logger = null;
28+
logger.info("hash: " + hashPassword(password)); // Safe - hashed
29+
logger.info("hash: " + sha256Digest(authToken)); // Safe - digested
30+
logger.info("enc: " + encryptValue(password)); // Safe - encrypted
31+
logger.info("pw: " + password); // $ Alert // not hashed
32+
}
33+
2534
// Tests for false positive exclusions: variables with "token" or "secret" in the name
2635
// that do not hold sensitive data.
2736
void testFalsePositiveExclusions(
@@ -72,4 +81,8 @@ void testTruePositives(String accessToken, String clientSecret, String apiSecret
7281
logger.info("token: " + refreshToken); // $ Alert
7382
logger.info("value: " + secretValue); // $ Alert
7483
}
84+
85+
static String hashPassword(String input) { return input; }
86+
static String sha256Digest(String input) { return input; }
87+
static String encryptValue(String input) { return input; }
7588
}

0 commit comments

Comments
 (0)