Skip to content

Commit 46ef020

Browse files
committed
Remove secretQuestion from FP exclusion list
secretQuestion is ambiguous: it could be the question text (not sensitive) or a security question answer. Worse, the regex secrets?(question) also matches secretQuestionAnswer, which is clearly sensitive. Drop it to avoid false negatives.
1 parent 20cfe29 commit 46ef020

File tree

3 files changed

+34
-35
lines changed

3 files changed

+34
-35
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ string getCommonSensitiveInfoFPRegex() {
6363
or
6464
// Secret metadata (secret followed by a non-value descriptor)
6565
result =
66-
"(?i).*secrets?(name|id|version|ref|arn|path|type|label|description|question|manager|client|provider|store|factory|properties).*"
66+
"(?i).*secrets?(name|id|version|ref|arn|path|type|label|description|manager|client|provider|store|factory|properties).*"
6767
}
6868

6969
/** An expression that might contain sensitive data. */

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

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -3,29 +3,29 @@
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:67:21:67:43 | ... + ... | Test.java:67:33:67:43 | accessToken : String | Test.java:67:21:67:43 | ... + ... | This $@ is written to a log file. | Test.java:67:33:67:43 | accessToken | potentially sensitive information |
7-
| Test.java:68:21:68:45 | ... + ... | Test.java:68:34:68:45 | clientSecret : String | Test.java:68:21:68:45 | ... + ... | This $@ is written to a log file. | Test.java:68:34:68:45 | clientSecret | potentially sensitive information |
8-
| Test.java:69:21:69:42 | ... + ... | Test.java:69:34:69:42 | apiSecret : String | Test.java:69:21:69:42 | ... + ... | This $@ is written to a log file. | Test.java:69:34:69:42 | apiSecret | potentially sensitive information |
9-
| Test.java:70:21:70:44 | ... + ... | Test.java:70:33:70:44 | sessionToken : String | Test.java:70:21:70:44 | ... + ... | This $@ is written to a log file. | Test.java:70:33:70:44 | sessionToken | potentially sensitive information |
10-
| Test.java:71:21:71:43 | ... + ... | Test.java:71:33:71:43 | bearerToken : String | Test.java:71:21:71:43 | ... + ... | This $@ is written to a log file. | Test.java:71:33:71:43 | bearerToken | potentially sensitive information |
11-
| Test.java:72:21:72:39 | ... + ... | Test.java:72:31:72:39 | secretKey : String | Test.java:72:21:72:39 | ... + ... | This $@ is written to a log file. | Test.java:72:31:72:39 | secretKey | potentially sensitive information |
12-
| Test.java:73:21:73:44 | ... + ... | Test.java:73:33:73:44 | refreshToken : String | Test.java:73:21:73:44 | ... + ... | This $@ is written to a log file. | Test.java:73:33:73:44 | refreshToken | potentially sensitive information |
13-
| Test.java:74:21:74:43 | ... + ... | Test.java:74:33:74:43 | secretValue : String | Test.java:74:21:74:43 | ... + ... | This $@ is written to a log file. | Test.java:74:33:74:43 | secretValue | 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 |
1414
edges
1515
| Test.java:11:46:11:53 | password : String | Test.java:11:21:11:53 | ... + ... | provenance | Sink:MaD:2 |
1616
| Test.java:12:44:12:52 | authToken : String | Test.java:12:22:12:52 | ... + ... | provenance | Sink:MaD:1 |
1717
| Test.java:21:44:21:52 | authToken : String | Test.java:21:44:21:67 | substring(...) : String | provenance | MaD:3 |
1818
| Test.java:21:44:21:67 | substring(...) : String | Test.java:21:22:21:75 | ... + ... | provenance | Sink:MaD:1 |
1919
| Test.java:22:44:22:52 | authToken : String | Test.java:22:44:22:67 | substring(...) : String | provenance | MaD:3 |
2020
| Test.java:22:44:22:67 | substring(...) : String | Test.java:22:22:22:75 | ... + ... | provenance | Sink:MaD:1 |
21-
| Test.java:67:33:67:43 | accessToken : String | Test.java:67:21:67:43 | ... + ... | provenance | Sink:MaD:2 |
22-
| Test.java:68:34:68:45 | clientSecret : String | Test.java:68:21:68:45 | ... + ... | provenance | Sink:MaD:2 |
23-
| Test.java:69:34:69:42 | apiSecret : String | Test.java:69:21:69:42 | ... + ... | provenance | Sink:MaD:2 |
24-
| Test.java:70:33:70:44 | sessionToken : String | Test.java:70:21:70:44 | ... + ... | provenance | Sink:MaD:2 |
25-
| Test.java:71:33:71:43 | bearerToken : String | Test.java:71:21:71:43 | ... + ... | provenance | Sink:MaD:2 |
26-
| Test.java:72:31:72:39 | secretKey : String | Test.java:72:21:72:39 | ... + ... | provenance | Sink:MaD:2 |
27-
| Test.java:73:33:73:44 | refreshToken : String | Test.java:73:21:73:44 | ... + ... | provenance | Sink:MaD:2 |
28-
| Test.java:74:33:74:43 | secretValue : String | Test.java:74:21:74:43 | ... + ... | provenance | Sink:MaD:2 |
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 |
2929
models
3030
| 1 | Sink: org.apache.logging.log4j; Logger; true; error; (String); ; Argument[0]; log-injection; manual |
3131
| 2 | Sink: org.apache.logging.log4j; Logger; true; info; (String); ; Argument[0]; log-injection; manual |
@@ -41,20 +41,20 @@ nodes
4141
| Test.java:22:22:22:75 | ... + ... | semmle.label | ... + ... |
4242
| Test.java:22:44:22:52 | authToken : String | semmle.label | authToken : String |
4343
| Test.java:22:44:22:67 | substring(...) : String | semmle.label | substring(...) : String |
44-
| Test.java:67:21:67:43 | ... + ... | semmle.label | ... + ... |
45-
| Test.java:67:33:67:43 | accessToken : String | semmle.label | accessToken : String |
46-
| Test.java:68:21:68:45 | ... + ... | semmle.label | ... + ... |
47-
| Test.java:68:34:68:45 | clientSecret : String | semmle.label | clientSecret : String |
48-
| Test.java:69:21:69:42 | ... + ... | semmle.label | ... + ... |
49-
| Test.java:69:34:69:42 | apiSecret : String | semmle.label | apiSecret : String |
50-
| Test.java:70:21:70:44 | ... + ... | semmle.label | ... + ... |
51-
| Test.java:70:33:70:44 | sessionToken : String | semmle.label | sessionToken : String |
52-
| Test.java:71:21:71:43 | ... + ... | semmle.label | ... + ... |
53-
| Test.java:71:33:71:43 | bearerToken : String | semmle.label | bearerToken : String |
54-
| Test.java:72:21:72:39 | ... + ... | semmle.label | ... + ... |
55-
| Test.java:72:31:72:39 | secretKey : String | semmle.label | secretKey : String |
56-
| Test.java:73:21:73:44 | ... + ... | semmle.label | ... + ... |
57-
| Test.java:73:33:73:44 | refreshToken : String | semmle.label | refreshToken : String |
58-
| Test.java:74:21:74:43 | ... + ... | semmle.label | ... + ... |
59-
| Test.java:74:33:74:43 | secretValue : String | semmle.label | secretValue : 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 |
6060
subpaths

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ void testFalsePositiveExclusions(
2929
String tokenType, String tokenEndpoint, String tokenCount, String tokenUrl,
3030
String tokenIndex, String tokenLength, String tokenName, String tokenId,
3131
String secretName, String secretId, String secretVersion, String secretArn,
32-
String secretPath, String secretType, String secretQuestion,
32+
String secretPath, String secretType,
3333
String secretManager, String secretProperties
3434
) {
3535
Logger logger = null;
@@ -54,7 +54,6 @@ void testFalsePositiveExclusions(
5454
logger.info("arn: " + secretArn); // Safe
5555
logger.info("path: " + secretPath); // Safe
5656
logger.info("type: " + secretType); // Safe
57-
logger.info("question: " + secretQuestion); // Safe
5857
logger.info("manager: " + secretManager); // Safe
5958
logger.info("properties: " + secretProperties); // Safe
6059
}

0 commit comments

Comments
 (0)