mirror of
https://github.com/github/codeql.git
synced 2026-05-14 11:19:27 +02:00
Merge pull request #21650 from MarkLee131/fix/sensitive-log-fp-regex
Java: reduce false positives in sensitive-log
This commit is contained in:
Owen Mansel-Chan
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: minorAnalysis
|
||||
---
|
||||
* The `java/sensitive-log` query now excludes additional common variable naming patterns that do not hold sensitive data, reducing false positives. This includes pagination/iteration tokens (`nextToken`, `pageToken`, `continuationToken`), token metadata (`tokenType`, `tokenEndpoint`, `tokenCount`), and secret metadata (`secretName`, `secretId`, `secretVersion`).
|
||||
@@ -40,14 +40,26 @@ string getCommonSensitiveInfoRegex() {
|
||||
|
||||
/**
|
||||
* Gets a regular expression for matching common names of variables that
|
||||
* indicate the value being held does not contains sensitive information,
|
||||
* indicate the value being held does not contain sensitive information,
|
||||
* but is a false positive for `getCommonSensitiveInfoRegex`.
|
||||
*
|
||||
* - "tokenizer" is often used for java.util.StringTokenizer.
|
||||
* - "tokenImage" appears in parser code generated by JavaCC.
|
||||
* - Pagination/iteration tokens: "nextToken" (AWS SDK), "pageToken" (GCP), etc.
|
||||
* - Token metadata: "tokenType" (OAuth), "tokenEndpoint" (OIDC), "tokenCount", etc.
|
||||
* - Secret metadata: "secretName" (K8s/AWS), "secretId" (Azure), "secretVersion", etc.
|
||||
*/
|
||||
string getCommonSensitiveInfoFPRegex() {
|
||||
result = "(?i).*(null|tokenizer).*" or result = "tokenImage"
|
||||
result =
|
||||
[
|
||||
"(?i).*(null|tokenizer).*", "tokenImage",
|
||||
// Pagination/iteration tokens (e.g., AWS SDK pagination cursors, parser tokens)
|
||||
"(?i).*(next|previous|current|page|continuation|cursor)tokens?.*",
|
||||
// Token metadata/infrastructure (token followed by a non-value descriptor)
|
||||
"(?i).*tokens?(type|kind|count|index|position|length|offset|endpoint|url|uri|bucket|rate|delimiter|separator|format|number|name|id|prefix|suffix|pattern|class|style).*",
|
||||
// Secret metadata (secret followed by a non-value descriptor)
|
||||
"(?i).*secrets?(name|id|version|ref|arn|path|type|label|description|manager|client|provider|store|factory|properties).*"
|
||||
]
|
||||
}
|
||||
|
||||
/** An expression that might contain sensitive data. */
|
||||
|
||||
@@ -3,6 +3,14 @@
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
| 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 |
|
||||
edges
|
||||
| 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 |
|
||||
@@ -10,6 +18,14 @@ edges
|
||||
| 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 |
|
||||
| Test.java:66:33:66:43 | accessToken : String | Test.java:66:21:66:43 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:67:34:67:45 | clientSecret : String | Test.java:67:21:67:45 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:68:34:68:42 | apiSecret : String | Test.java:68:21:68:42 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:69:33:69:44 | sessionToken : String | Test.java:69:21:69:44 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:70:33:70:43 | bearerToken : String | Test.java:70:21:70:43 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:71:31:71:39 | secretKey : String | Test.java:71:21:71:39 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:72:33:72:44 | refreshToken : String | Test.java:72:21:72:44 | ... + ... | provenance | Sink:MaD:2 |
|
||||
| Test.java:73:33:73:43 | secretValue : String | Test.java:73:21:73:43 | ... + ... | provenance | Sink:MaD:2 |
|
||||
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 |
|
||||
@@ -25,4 +41,20 @@ nodes
|
||||
| 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 |
|
||||
| Test.java:66:21:66:43 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:66:33:66:43 | accessToken : String | semmle.label | accessToken : String |
|
||||
| Test.java:67:21:67:45 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:67:34:67:45 | clientSecret : String | semmle.label | clientSecret : String |
|
||||
| Test.java:68:21:68:42 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:68:34:68:42 | apiSecret : String | semmle.label | apiSecret : String |
|
||||
| Test.java:69:21:69:44 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:69:33:69:44 | sessionToken : String | semmle.label | sessionToken : String |
|
||||
| Test.java:70:21:70:43 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:70:33:70:43 | bearerToken : String | semmle.label | bearerToken : String |
|
||||
| Test.java:71:21:71:39 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:71:31:71:39 | secretKey : String | semmle.label | secretKey : String |
|
||||
| Test.java:72:21:72:44 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:72:33:72:44 | refreshToken : String | semmle.label | refreshToken : String |
|
||||
| Test.java:73:21:73:43 | ... + ... | semmle.label | ... + ... |
|
||||
| Test.java:73:33:73:43 | secretValue : String | semmle.label | secretValue : String |
|
||||
subpaths
|
||||
|
||||
@@ -21,4 +21,55 @@ class Test {
|
||||
logger.error("Auth failed for: " + authToken.substring(1,5) + "..."); // $ Alert
|
||||
logger.error("Auth failed for: " + authToken.substring(0,8) + "..."); // $ Alert
|
||||
}
|
||||
|
||||
// Tests for false positive exclusions: variables with "token" or "secret" in the name
|
||||
// that do not hold sensitive data.
|
||||
void testFalsePositiveExclusions(
|
||||
String nextToken, String pageToken, String continuationToken, String cursorToken,
|
||||
String tokenType, String tokenEndpoint, String tokenCount, String tokenUrl,
|
||||
String tokenIndex, String tokenLength, String tokenName, String tokenId,
|
||||
String secretName, String secretId, String secretVersion, String secretArn,
|
||||
String secretPath, String secretType,
|
||||
String secretManager, String secretProperties
|
||||
) {
|
||||
Logger logger = null;
|
||||
// Pagination/iteration tokens (e.g., AWS SDK, GCP, Azure pagination cursors)
|
||||
logger.info("cursor: " + nextToken); // Safe
|
||||
logger.info("cursor: " + pageToken); // Safe
|
||||
logger.info("cursor: " + continuationToken); // Safe
|
||||
logger.info("cursor: " + cursorToken); // Safe
|
||||
// Token metadata (e.g., OAuth token type, OIDC discovery endpoint)
|
||||
logger.info("type: " + tokenType); // Safe
|
||||
logger.info("endpoint: " + tokenEndpoint); // Safe
|
||||
logger.info("count: " + tokenCount); // Safe
|
||||
logger.info("url: " + tokenUrl); // Safe
|
||||
logger.info("index: " + tokenIndex); // Safe
|
||||
logger.info("length: " + tokenLength); // Safe
|
||||
logger.info("name: " + tokenName); // Safe
|
||||
logger.info("id: " + tokenId); // Safe
|
||||
// Secret metadata (e.g., K8s secret name, AWS Secrets Manager identifiers)
|
||||
logger.info("name: " + secretName); // Safe
|
||||
logger.info("id: " + secretId); // Safe
|
||||
logger.info("version: " + secretVersion); // Safe
|
||||
logger.info("arn: " + secretArn); // Safe
|
||||
logger.info("path: " + secretPath); // Safe
|
||||
logger.info("type: " + secretType); // Safe
|
||||
logger.info("manager: " + secretManager); // Safe
|
||||
logger.info("properties: " + secretProperties); // Safe
|
||||
}
|
||||
|
||||
// These should still be flagged as sensitive
|
||||
void testTruePositives(String accessToken, String clientSecret, String apiSecret,
|
||||
String sessionToken, String bearerToken, String secretKey,
|
||||
String refreshToken, String secretValue) {
|
||||
Logger logger = null;
|
||||
logger.info("token: " + accessToken); // $ Alert
|
||||
logger.info("secret: " + clientSecret); // $ Alert
|
||||
logger.info("secret: " + apiSecret); // $ Alert
|
||||
logger.info("token: " + sessionToken); // $ Alert
|
||||
logger.info("token: " + bearerToken); // $ Alert
|
||||
logger.info("key: " + secretKey); // $ Alert
|
||||
logger.info("token: " + refreshToken); // $ Alert
|
||||
logger.info("value: " + secretValue); // $ Alert
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user