Data flow: Add FeatureEscapesSourceCallContext(OrEqualSourceSinkCallContext) flow feature#21404
Open
hvitved wants to merge 4 commits intogithub:mainfrom
Open
Data flow: Add FeatureEscapesSourceCallContext(OrEqualSourceSinkCallContext) flow feature#21404hvitved wants to merge 4 commits intogithub:mainfrom
FeatureEscapesSourceCallContext(OrEqualSourceSinkCallContext) flow feature#21404hvitved wants to merge 4 commits intogithub:mainfrom
Conversation
github-actions
bot
added
Rust
hvitved
force-pushed
the
dataflow/no-enclosing-stack-flow-feature
branch
2 times, most recently
from
2e494c1 to
79dfb75
Compare
hvitved
changed the title
FeatureNoEnclosingCallStack(Strict) featureFeatureEscapesSourceCallContext(OrEqualSourceSinkCallContext) flow feature
hvitved
force-pushed
the
dataflow/no-enclosing-stack-flow-feature
branch
from
b4f38a0 to
04fdc29
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…Context)` flow feature
…CallContextOrEqualSourceSinkCallContext`
hvitved
force-pushed
the
dataflow/no-enclosing-stack-flow-feature
branch
from
04fdc29 to
4474e25
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the shared global data flow implementation with two new flow features to constrain results based on whether flow escapes the source call context, and updates the Rust AccessAfterLifetime query to use these features for simpler and faster filtering.
Changes:
- Add
FeatureEscapesSourceCallContextandFeatureEscapesSourceCallContextOrEqualSourceSinkCallContextas new flow configuration features in shared dataflow internals. - Update shared dataflow internals to track and enforce the new “escapes source call context” constraint during flow computation.
- Simplify Rust
AccessAfterLifetime.qlby relying on the new flow feature and adjust Rust tests/expected output accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/dataflow/codeql/dataflow/internal/DataFlowImplStage1.qll | Adds Stage 1 predicate plumbing for the new escape-context feature. |
| shared/dataflow/codeql/dataflow/internal/DataFlowImplCommon.qll | Defines the new flow feature types and public feature classes. |
| shared/dataflow/codeql/dataflow/internal/DataFlowImpl.qll | Implements summary-context handling to enforce the new feature during dataflow. |
| shared/dataflow/change-notes/2026-03-04-flow-feature-escapes-source-call-context.md | Adds changelog entry documenting the new flow features. |
| rust/ql/src/queries/security/CWE-825/AccessAfterLifetime.ql | Switches the query to use the new flow feature and removes ad-hoc filtering logic. |
| rust/ql/test/query-tests/security/CWE-825/lifetime.rs | Updates inline expectations to reflect the new/changed result. |
| rust/ql/test/query-tests/security/CWE-825/AccessAfterLifetime.expected | Updates expected output to match new analysis results. |
| } | ||
|
|
||
| /** | ||
| * A flow configuration feature that is the disjuction of `FeatureEscapesSourceCallContext` |
There was a problem hiding this comment.
Typo in comment: "disjuction" should be "disjunction".
Suggested change
| * A flow configuration feature that is the disjuction of `FeatureEscapesSourceCallContext` | |
| * A flow configuration feature that is the disjunction of `FeatureEscapesSourceCallContext` |
|
|
||
| predicate hasSourceCallCtx(); | ||
|
|
||
| predicate hasFeatureEscapesSourceCallContext(boolean nonEmpty); |
There was a problem hiding this comment.
The boolean parameter name in the signature uses nonEmpty, but the implementation and the underlying feature use strict. Consider renaming the parameter in the signature to strict for consistency/readability.
aschackmull
reviewed
Mar 4, 2026
| TSummaryCtxSome(ParamNd p, Typ t, Ap ap, TypOption stored) { | ||
| fwdFlowInFlowThrough(p, _, t, ap, stored) | ||
| } | ||
| TSummaryCtxSome(ParamNd p, Typ t, Ap ap, TypOption stored, Boolean mustReturn) { |
Contributor
There was a problem hiding this comment.
Suggested change
| TSummaryCtxSome(ParamNd p, Typ t, Ap ap, TypOption stored, Boolean mustReturn) { | |
| TSummaryCtxSome(ParamNd p, Typ t, Ap ap, TypOption stored, boolean mustReturn) { |
aschackmull
reviewed
Mar 4, 2026
Comment on lines
+1008
to
+1010
| exists(SummaryCtx summaryCtx | | ||
| FwdFlowInNoThrough::fwdFlowIn(_, _, _, p, _, innercc, summaryCtx, t, ap, stored, _) and | ||
| summaryCtx.isValidForFlowInNoThrough(innerSummaryCtx) |
Contributor
There was a problem hiding this comment.
Let's rename this to include the "outer" prefix to make it more clear.
Suggested change
| exists(SummaryCtx summaryCtx | | |
| FwdFlowInNoThrough::fwdFlowIn(_, _, _, p, _, innercc, summaryCtx, t, ap, stored, _) and | |
| summaryCtx.isValidForFlowInNoThrough(innerSummaryCtx) | |
| exists(SummaryCtx outerSummaryCtx | | |
| FwdFlowInNoThrough::fwdFlowIn(_, _, _, p, _, innercc, outerSummaryCtx, t, ap, stored, _) and | |
| outerSummaryCtx.isValidForFlowInNoThrough(innerSummaryCtx) |
Contributor
There was a problem hiding this comment.
Or maybe not, I think skipping the prefix is more consistent with what we've done elsewhere. See also below.
aschackmull
reviewed
Mar 4, 2026
| ) { | ||
| FwdFlowInThrough::fwdFlowIn(_, _, _, p, _, innercc, _, t, ap, stored, _) | ||
| exists(SummaryCtx outerSummaryCtx | |
Contributor
There was a problem hiding this comment.
Skip the "outer" prefix as per above comment.
aschackmull
reviewed
Mar 4, 2026
| @@ -46,10 +47,11 @@ module MakeImplCommon<LocationSig Location, InputSig<Location> Lang> { | |||
| } | |||
| } | |||
|
|
|||
| private newtype TFlowFeature = | |||
| newtype TFlowFeature = | |||
aschackmull
reviewed
Mar 4, 2026
Contributor
aschackmull
left a comment
aschackmull
left a comment
There was a problem hiding this comment.
A few nits, otherwise LGTM.
hvitved
force-pushed
the
dataflow/no-enclosing-stack-flow-feature
branch
from
2536be0 to
db491fc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds two new flow features
FeatureEscapesSourceCallContextandFeatureEscapesSourceCallContextOrEqualSourceSinkCallContext.The former implies that the sink must be reached from the source by escaping the source call context, that is, flow must either return from the callable containing the source or use a jump-step before reaching the sink. The latter is the disjunction of the former and the existing
FeatureEqualSourceSinkCallContextflow feature.This allows us to greatly simplify the Rust query
AccessAfterLifetime.ql, in addition to improving performance and precision of said query, since filtering of results now happen as part of the data flow computation instead of an ad-hoc after-the-fact filter.DCA for all languages except Rust is uneventful, and for Rust it shows that we achieve a significant speedup.