Java: mass enable diff-informed data flow + none() overrides
#19795
Conversation
An auto-generated patch that enables diff-informed data flow in the obvious cases. Builds on github#18346 and github/codeql-patch#88
There was a problem hiding this comment.
Pull Request Overview
This PR auto-enables diff-informed data flow for several Java security queries by adding the required overrides in their DataFlow configurations.
- Adds
observeDiffInformedIncrementalMode()override returningany()in eight config modules. - Adds
getASelectedSourceLocation()override returningnone()in the APK installation config. - Covers queries that were omitted in the previous diff-informed enablement pass.
Reviewed Changes
Copilot reviewed 8 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| UnsafeAndroidAccessQuery.qll | Added observeDiffInformedIncrementalMode() override |
| SensitiveUiQuery.qll | Added observeDiffInformedIncrementalMode() override |
| InsecureBasicAuthQuery.qll | Added observeDiffInformedIncrementalMode() override |
| HttpsUrlsQuery.qll | Added observeDiffInformedIncrementalMode() override |
| HardcodedCredentialsSourceCallQuery.qll | Added observeDiffInformedIncrementalMode() override |
| HardcodedCredentialsApiCallQuery.qll | Added observeDiffInformedIncrementalMode() override |
| ArbitraryApkInstallationQuery.qll | Added observeDiffInformedIncrementalMode() and getASelectedSourceLocation() overrides |
| TextFieldTrackingConfig in SensitiveUiQuery.qll (module excerpt) | Added observeDiffInformedIncrementalMode() override |
Comments suppressed due to low confidence (2)
java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll:29
- [nitpick] If only
getASelectedSourceLocationis overridden, add a comment explaining whygetASelectedSinkLocationisn’t needed, to clarify intent for future readers.
Location getASelectedSourceLocation(DataFlow::Node sink) { none() }
java/ql/lib/semmle/code/java/security/ArbitraryApkInstallationQuery.qll:27
- Consider adding or updating tests to verify that the query behaves correctly under diff-informed incremental mode, ensuring no regressions are introduced.
predicate observeDiffInformedIncrementalMode() { any() }
| @@ -15,6 +15,8 @@ module FetchUntrustedResourceConfig implements DataFlow::ConfigSig { | |||
| predicate isSink(DataFlow::Node sink) { sink instanceof UrlResourceSink } | |||
|
|
|||
| predicate isBarrier(DataFlow::Node sanitizer) { sanitizer instanceof RequestForgerySanitizer } | |||
|
|
|||
| predicate observeDiffInformedIncrementalMode() { any() } | |||
There was a problem hiding this comment.
This observeDiffInformedIncrementalMode() override is duplicated across many config modules. Consider providing a default implementation in DataFlow::ConfigSig to reduce boilerplate.
Copilot uses AI. Check for mistakes.
| @@ -17,6 +17,8 @@ module BasicAuthFlowConfig implements DataFlow::ConfigSig { | |||
| predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) { | |||
| any(HttpUrlsAdditionalTaintStep c).step(node1, node2) | |||
| } | |||
|
|
|||
| predicate observeDiffInformedIncrementalMode() { any() } | |||
There was a problem hiding this comment.
[nitpick] For consistency with surrounding modules, ensure there’s a blank line after this predicate and before the closing brace, matching the established file style.
| predicate observeDiffInformedIncrementalMode() { any() } | |
| predicate observeDiffInformedIncrementalMode() { any() } |
Copilot uses AI. Check for mistakes.
An auto-generated patch that enables diff-informed data flow in the obvious cases.
observeDiffInformedIncrementalMode() { any() }override to queries that don't select a location other than a dataflow source or sink.getASelected{Source,Sink}Location() { none() }override to queries that select a dataflow source or sink as a location, but not both.Java has previously had a round of diff-informed query enablements, but it seems some queries were missed in the first round.