Skip to content

Java: whitelist variable name tokenImage for java/sensitive-log as it's used in code generated by JavaCC#16028

Merged
owen-mc merged 5 commits into
github:mainfrom
owen-mc:java/sensitive-log-whitelist-tokenimage
Mar 25, 2024
Merged

Java: whitelist variable name tokenImage for java/sensitive-log as it's used in code generated by JavaCC#16028
owen-mc merged 5 commits into
github:mainfrom
owen-mc:java/sensitive-log-whitelist-tokenimage

Conversation

@owen-mc
Copy link
Copy Markdown
Contributor

@owen-mc owen-mc commented Mar 23, 2024

Note that the test imitates what is done in code generated by JavaCC.

I've whitelisted just exact matches to tokenImage. A slightly broader approach might be to copy this line from the experimental query java/sensitive-query-with-get and whitelist all variable names starting with token.

@owen-mc owen-mc requested a review from a team as a code owner March 23, 2024 21:38
@github-actions github-actions Bot added the Java label Mar 23, 2024
Comment thread java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll Fixed
Comment thread java/ql/src/change-notes/2024-03-24-sensitive-log-whitelist-tokenimage.md Outdated
Comment thread java/ql/lib/semmle/code/java/security/SensitiveLoggingQuery.qll Outdated
Co-authored-by: Chris Smowton <smowton@github.com>
@owen-mc owen-mc force-pushed the java/sensitive-log-whitelist-tokenimage branch from 9d43e78 to ac6c4ad Compare March 24, 2024 20:21
@owen-mc
Copy link
Copy Markdown
Contributor Author

owen-mc commented Mar 24, 2024

I did an MRVA run on the top 1,000 java repos to see what results this PR was removing. There were 11 repos with results, including ones with 100, 314, 768 and 2,640 results. I checked the first result from each repo and confirmed it was due to code from JavaCC. So I think that confirms that they are all FPs and this PR is good to merge. It's also good to see that we are removing a source of FPs that can cause thousands of FPs in one repo.

@smowton
Copy link
Copy Markdown
Contributor

smowton commented Mar 25, 2024

That does sound like CodeQL would be borderline unusable for such people!

@owen-mc owen-mc merged commit f2db9ce into github:main Mar 25, 2024
@owen-mc owen-mc deleted the java/sensitive-log-whitelist-tokenimage branch March 25, 2024 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants