Uh oh!
There was an error while loading. Please reload this page.
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() }
| 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.
| 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.
| predicateobserveDiffInformedIncrementalMode(){any()} | |
| predicateobserveDiffInformedIncrementalMode(){any()} |
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.