Skip to content

Java: CWE-532 sensitive info logging #3090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 12 commits into from
May 14, 2020
Merged

Java: CWE-532 sensitive info logging #3090

merged 12 commits into from
May 14, 2020

Conversation

luchua-bc
Copy link
Contributor

Third-party logging utilities like Log4J and SLF4J are widely used in Java projects. This PR adds a codeql check for writing sensitive information to debug logs.

@luchua-bc luchua-bc requested review from felicitymay and a team as code owners March 18, 2020 20:04
@ghost
Copy link

ghost commented Mar 18, 2020 •
edited by ghost
Loading

CLA assistant check
All committers have signed the CLA.

@luchua-bc luchua-bc mentioned this pull request Mar 19, 2020
Closed
@luchua-bc luchua-bc force-pushed the java-insert-sensitive-info-into-log branch from 329ba28 to d932770 Compare March 20, 2020 12:20
@xcorail xcorail mentioned this pull request Mar 27, 2020
Closed
1 task
@yo-h yo-h added the Java label May 2, 2020
felicitymay
felicitymay reviewed May 4, 2020
View reviewed changes
Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for including a query help file with your new query. It makes it much easier for everyone to understand what your query does 😄

I've added a few suggested changes for typos and clarity.

@luchua-bc @felicitymay
Update java/ql/src/experimental/CWE-532/SensitiveInfoLog.qhelp
a256065 
Co-authored-by: Felicity Chapman <felicitymay@github.com>
@luchua-bc luchua-bc requested a review from a team as a code owner May 4, 2020 10:57
luchua-bc and others added 2 commits May 4, 2020 06:58
@luchua-bc @felicitymay
Update java/ql/src/experimental/CWE-532/SensitiveInfoLog.ql
a6c9c51 
Co-authored-by: Felicity Chapman <felicitymay@github.com>
@luchua-bc
The query help builder will interpret and automatically add this refe…
5c803b7 
…rence so this isn't needed here.
aschackmull
aschackmull requested changes May 11, 2020
View reviewed changes
Copy link
Contributor

@aschackmull aschackmull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some inline comments. Also, the ql file needs to be autoformatted to pass PR checks. If you have tested this query on some number of projects it would be nice if you could share a bit of what you learned from looking through the results.

luchua-bc added 2 commits May 12, 2020 22:57
@luchua-bc
Change string concatenation in the source to TaintTracking::Configura…
491b67e 
…tion
@luchua-bc
Fine tuning criteria
ffd442a 
1. Change the regex pattern from variable contains "url" to variable starts with "url"
2. Add the logging trace method to sink
@aschackmull aschackmull merged commit 5c9fb23 into github:master May 14, 2020
@luchua-bc luchua-bc deleted the java-insert-sensitive-info-into-log branch May 14, 2020 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JLLeitschuh JLLeitschuh JLLeitschuh left review comments

@felicitymay felicitymay felicitymay left review comments

@aschackmull aschackmull aschackmull approved these changes

Assignees
No one assigned
Labels
Java
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@luchua-bc @JLLeitschuh @felicitymay @aschackmull @yo-h