Skip to content

Java: CWE-532 sensitive info logging - #3090

Merged
aschackmull merged 12 commits into
github:masterfrom
luchua-bc:java-insert-sensitive-info-into-log
May 14, 2020
Merged

Java: CWE-532 sensitive info logging#3090
aschackmull merged 12 commits into
github:masterfrom
luchua-bc:java-insert-sensitive-info-into-log

Conversation

@luchua-bc

@luchua-bcluchua-bc commented Mar 18, 2020

Copy link
Copy Markdown
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 a team and felicitymay as code ownersMarch 18, 2020 20:04
@ghost

ghost commented Mar 18, 2020•
edited by ghost
Loading

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@luchua-bcluchua-bc mentioned this pull request Mar 19, 2020
Closed
@luchua-bc
luchua-bcforce-pushed the java-insert-sensitive-info-into-log branch from 329ba28 to d932770CompareMarch 20, 2020 12:20
@xcorailxcorail mentioned this pull request Mar 27, 2020
Closed
1 task
JLLeitschuh
JLLeitschuh reviewed Mar 27, 2020
View reviewed changes
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
JLLeitschuh
JLLeitschuh reviewed Mar 27, 2020
View reviewed changes
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
@yo-hyo-h added the Java label May 2, 2020
felicitymay
felicitymay reviewed May 4, 2020
View reviewed changes

@felicitymayfelicitymay left a comment

Copy link
Copy Markdown
Contributor

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.

Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.qhelp Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.qhelp Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
@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 ownerMay 4, 2020 10:57
luchua-bcand 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

@aschackmullaschackmull left a comment

Copy link
Copy Markdown
Contributor

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.

Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql
@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 requested changes May 13, 2020
View reviewed changes
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
Comment threadjava/ql/src/experimental/CWE-532/SensitiveInfoLog.ql Outdated
@aschackmull
aschackmull merged commit 5c9fb23 into github:masterMay 14, 2020
@luchua-bc
luchua-bc deleted the java-insert-sensitive-info-into-log branch May 14, 2020 11:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aschackmullaschackmullaschackmull approved these changes
+2 more reviewers
@JLLeitschuhJLLeitschuhJLLeitschuh left review comments
@felicitymayfelicitymayfelicitymay left review comments
Reviewers whose approvals may not affect merge requirements

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