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: make more queries diff-informed with getASelectedLocation #18340

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jbj
Copy link
Contributor

@jbj jbj commented Dec 20, 2024

This PR contains the uncontroversial parts of #17846, converting 9 data-flow configurations to be diff-informed either by using the concept of selected source/sink locations or by just marking them as diff-informed because I've checked that all use of the configuration is safe.

I recommend reviewing commit-by-commit.

Ideally, these changes should be tested with codeql test run --check-diff-informed. Most queries were not in shape for potentially failing that test, and I only converted one: UnsafeHostnameVerification.ql. Other queries either had no test cases with challenging location distributions, or they had no qlref-based tests. In particular, these three old-style inline expectations tests should be converted to qlref and post-processing:

java/ql/test/query-tests/security/CWE-295/InsecureTrustManager/InsecureTrustManagerTest.ql
java/ql/test/query-tests/security/CWE-925/ImproperIntentVerification.ql
java/ql/test/query-tests/security/CWE-730/PolynomialReDoS.ql

jbj added 10 commits December 20, 2024 11:22
This extension allows queries to be diff-informed even when the elements
they select are different from the sources and sinks found by data flow.
This commit also adds a test case that would fail under `codeql test run
--check-diff-informed` if not for the override of
`getASelectedSourceLocation`. There was no existing such test since all
the existing tests used anonymous classes whose location was on the same
line as the source.
This and other queries would also benefit from making `RegexFlow`
diff-informed. That will come later.
@jbj jbj added the no-change-note-required This PR does not need a change note label Dec 20, 2024
@jbj jbj requested review from aschackmull and Copilot December 20, 2024 12:11
@jbj jbj requested a review from a team as a code owner December 20, 2024 12:11

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 13 changed files in this pull request and generated no comments.

Files not reviewed (12)
  • java/ql/lib/semmle/code/java/security/BrokenCryptoAlgorithmQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/CommandLineQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/InsecureTrustManagerQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/RandomQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/SqlInjectionQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/TaintedPermissionsCheckQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/UnsafeHostnameVerificationQuery.qll: Language not supported
  • java/ql/lib/semmle/code/java/security/regexp/PolynomialReDoSQuery.qll: Language not supported
  • java/ql/test/query-tests/security/CWE-297/UnsafeHostnameVerification.expected: Language not supported
  • shared/dataflow/codeql/dataflow/DataFlow.qll: Language not supported
  • shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DataFlow Library Java no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant