-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Dataflow: Add flowFrom predicates to mirror flowTo. #20961
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces flowFrom predicates to the dataflow library to complement the existing flowTo predicates, enabling query optimizations by skipping transitive closure calculations. The change systematically updates queries across multiple languages to use these more efficient predicates where appropriate.
Key Changes:
- Added
flowFromandflowFromExprpredicates to the shared dataflow implementation - Updated 40+ queries across Swift, Rust, Ruby, Python, JavaScript, Java, Go, C#, and C++ to use
flowToinstead offlow(_, sink)andflowFrominstead offlow(source, _)
Reviewed changes
Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll | Adds flowFrom, flowFromLocal, and flowFromExpr predicates mirroring existing flowTo variants |
| swift/ql/lib/codeql/swift/security/CleartextTransmissionExtensions.qll | Replaces flow(_, this) with flowTo(this) |
| rust/ql/src/queries/security/CWE-614/InsecureCookie.ql | Replaces flow(_, sinkNode.getNode()) with flowTo(sinkNode.getNode()) |
| ruby/ql/src/queries/meta/TaintedNodes.ql | Replaces flow(_, node) with flowTo(node) |
| python/ql/src/meta/alerts/RemoteFlowSourcesReach.ql | Replaces flow(_, reachable) with flowTo(reachable) |
| python/ql/src/Security/CWE-020-ExternalAPIs/ExternalAPIs.qll | Replaces flow(_, this) with flowTo(this) |
| python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll | Replaces flow(_, urlPart) with flowTo(urlPart) |
| javascript/ql/test/tutorials/Introducing the JavaScript libraries/query17.qll | Replaces flow(_, sink) with flowTo(sink) |
| javascript/ql/test/library-tests/FlowSummary/test.ql | Replaces flow(_, result) with flowTo(result) |
| javascript/ql/src/experimental/Security/CWE-347/decodeJwtWithoutVerification.ql | Replaces flow(source.getNode(), _) with flowFrom(source.getNode()) |
| javascript/ql/lib/semmle/javascript/security/dataflow/ExternalAPIUsedWithUntrustedDataQuery.qll | Replaces multiple flow(_, sink) calls with flowTo(sink) |
| java/ql/test/library-tests/frameworks/android/taint-database/sinks.ql | Replaces flow(source, _) with flowFrom(source) |
| java/ql/test/library-tests/frameworks/android/taint-database/flowSteps.ql | Replaces flow(source, _) with flowFrom(source) |
| java/ql/test-kotlin2/library-tests/parameter-defaults/flowTest.ql | Replaces verbose flow(source, _) check with flowFromExpr(sl) |
| java/ql/test-kotlin1/library-tests/parameter-defaults/flowTest.ql | Replaces verbose flow(source, _) check with flowFromExpr(sl) |
| java/ql/src/experimental/Security/CWE/CWE-347/Auth0NoVerifier.ql | Replaces flow(source, _) with flowFrom(source) |
| java/ql/src/experimental/Security/CWE/CWE-346/UnvalidatedCors.ql | Replaces flow(source, _) with flowFrom(source) |
| java/ql/src/experimental/Security/CWE/CWE-295/JxBrowserWithoutCertValidation.ql | Replaces flow(src, _) with flowFrom(src) |
| java/ql/src/Security/CWE/CWE-200/AndroidWebViewSettingsAllowsContentAccess.ql | Replaces flow(source, _) with flowFrom(source) |
| java/ql/lib/semmle/code/java/security/ImproperIntentVerificationQuery.qll | Replaces flow(DataFlow::parameterNode(...), _) with flowFrom(DataFlow::parameterNode(...)) |
| java/ql/lib/semmle/code/java/regex/RegexFlowConfigs.qll | Replaces flow(DataFlow::exprNode(regex), _) with flowFromExpr(regex) |
| java/ql/lib/semmle/code/java/frameworks/jackson/JacksonSerializability.qll | Replaces flow(DataFlow::exprNode(result), _) with flowFromExpr(result) |
| java/ql/lib/semmle/code/java/frameworks/google/GoogleHttpClientApi.qll | Replaces verbose flow(DataFlow::exprNode(result), _) with flowFromExpr(result) |
| go/ql/src/experimental/CWE-321-V2/HardCodedKeys.ql | Replaces flow(n, _) with flowFrom(n) |
| go/ql/src/experimental/CWE-285/PamAuthBypass.ql | Replaces flow(source, _) with flowFrom(source) |
| go/ql/src/Security/CWE-918/RequestForgery.ql | Replaces flow(_, sink.getNode()) with flowTo(sink.getNode()) |
| go/ql/src/Security/CWE-601/OpenUrlRedirect.ql | Replaces flow(_, sink.getNode()) with flowTo(sink.getNode()) |
| go/ql/src/Security/CWE-352/ConstantOauth2State.ql | Replaces flow(authCodeUrlCall.getResult(), _) with flowFrom(authCodeUrlCall.getResult()) |
| go/ql/src/Security/CWE-020/MissingRegexpAnchor.ql | Replaces flow(source, _) with flowFrom(source) |
| go/ql/lib/semmle/go/security/UnsafeUnzipSymlink.qll | Replaces flow(getASimilarReadNode(node), _) with flowFrom(getASimilarReadNode(node)) |
| go/ql/lib/semmle/go/security/MissingJwtSignatureCheck.qll | Replaces flow(source, _) with flowFrom(source) |
| go/ql/lib/semmle/go/security/ExternalAPIs.qll | Replaces flow(_, this) with flowTo(this) |
| go/ql/lib/semmle/go/security/AllocationSizeOverflow.qll | Replaces flow(_, lenArg) with flowTo(lenArg) |
| csharp/ql/src/Security Features/CWE-614/CookieWithoutSecure.ql | Replaces verbose flow(creation, _) check with flowFromExpr(oc) |
| csharp/ql/src/Security Features/CWE-502/UnsafeDeserializationUntrustedInput.ql | Replaces flow(_, settingsCallArg) with flowTo(settingsCallArg) |
| csharp/ql/lib/semmle/code/csharp/security/dataflow/ReDoSQuery.qll | Replaces flow(_, DataFlow::exprNode(regexOperation.getPattern())) with flowToExpr(regexOperation.getPattern()) |
| csharp/ql/lib/semmle/code/csharp/security/dataflow/ExternalAPIsQuery.qll | Replaces flow(_, this) with flowTo(this) |
| csharp/ql/lib/semmle/code/csharp/frameworks/Sql.qll | Replaces flow(DataFlow::exprNode(this), _) with flowFromExpr(this) |
| cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql | Replaces flow(DataFlow::instructionNode(pai), _) with flowFrom(DataFlow::instructionNode(pai)) |
| cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql | Replaces flow(source, _) and flow(source.getNode(), _) with flowFrom variants |
| cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll | Replaces flow(_, this) with flowTo(this) |
| cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll | Replaces flow(_, this) with flowTo(this) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| exists(RegexOperation regexOperation | | ||
| // Exponential regex flows to the pattern argument | ||
| ExponentialRegexDataFlow::flow(_, DataFlow::exprNode(regexOperation.getPattern())) | ||
| ExponentialRegexDataFlow::flowToExpr(regexOperation.getPattern()) |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method flowToExpr is being called, but the newly added predicates in the shared dataflow implementation are flowFromExpr, not flowToExpr. This should be flowTo(DataFlow::exprNode(regexOperation.getPattern())) to match the pattern used in other files, or verify that flowToExpr exists.
| ExponentialRegexDataFlow::flowToExpr(regexOperation.getPattern()) | |
| ExponentialRegexDataFlow::flowTo(DataFlow::exprNode(regexOperation.getPattern())) |
owen-mc
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Go and Java LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rust, Swift LGTM (though we're still waiting for the Swift DCA run DCA has completed and is fine).
hvitved
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should perhaps add a QL4QL query for flagging calls to flow with _.
Using
flowTo(sink)instead offlow(_, sink)allows us to skip the TC calculation. And uses offlow(source, _)can be similarly rewritten to also skip the TC, except we didn't have theflowFrompredicate available. This PR rectifies this.Looking forward, the
flowToandflowFrompredicates also allowoverlay[local]variants, which can be useful as tools for building better incrementality.