Skip to content
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.

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

Java: Add modelling for Guava #4383

Merged
merged 11 commits into from Oct 26, 2020
Merged

Conversation

joefarebrother
Copy link
Contributor

@joefarebrother joefarebrother commented Oct 1, 2020

Adds taint steps for com.google.common.base.Strings, com.google.common.base.Splitter, and com.google.common.base.Joiner.
Also adds Strings.isNullOrEmpty as a null guard.

Part of https://github.com/github/codeql-java-team/issues/39.

Fixes #4355.

@joefarebrother joefarebrother requested a review from a team October 1, 2020 14:27
@github-actions github-actions bot added the Java label Oct 1, 2020
@aibaars
Copy link
Contributor

aibaars commented Oct 2, 2020

I think having one QLL files for each Java class is a bit overkill. It might be better to group things per package or per theme somehow.

@joefarebrother
Copy link
Contributor Author

I think having one QLL files for each Java class is a bit overkill. It might be better to group things per package or per theme somehow.

Good point, all of these could go under something like StringUtils.qll

@aschackmull
Copy link
Contributor

The additional Guava modelling is worth a change note.

@@ -0,0 +1,2 @@
lgtm,codescanning
* Some methods of the [Guava](https://guava.dev/) framework have been added as flow steps (specifically those of the [Splitter](https://guava.dev/releases/29.0-jre/api/docs/com/google/common/base/Splitter.html), [Joiner](https://guava.dev/releases/29.0-jre/api/docs/com/google/common/base/Joiner.html), and [Strings](https://guava.dev/releases/29.0-jre/api/docs/com/google/common/base/Strings.html) classes), which may lead to more results from the security queries.
Copy link
Contributor

Choose a reason for hiding this comment

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

Guava 30.0 has just been released. Would it make sense to adjust the links and the stubs?
Based on the changelog it looks like none of the classes covered by this pull request changed.

joefarebrother and others added 2 commits October 19, 2020 11:16
Co-authored-by: Marcono1234 <Marcono1234@users.noreply.github.com>
@joefarebrother joefarebrother merged commit 2050f82 into github:main Oct 26, 2020
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.

LGTM.com - false positive: Java variable cant be null after !Strings.isNullOrEmpty check
5 participants