-
Notifications
You must be signed in to change notification settings - Fork 1.7k
C#: Diff-informed queries: phase 3 (non-trivial locations) #20074
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
base: main
Are you sure you want to change the base?
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 enables diff-informed mode on C# queries that select locations other than dataflow source or sink by adding the observeDiffInformedIncrementalMode()
predicate and custom location overrides where needed. This is the final step in mass-enabling diff-informed queries across all languages.
- Adds
observeDiffInformedIncrementalMode()
predicate to enable diff-informed mode on data flow configurations - Implements custom location selection logic via
getASelectedSinkLocation()
for queries that report locations different from sink locations - Distinguishes between primary configurations (enabled with
any()
) and secondary configurations (disabled withnone()
)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
HardcodedConnectionString.ql | Enables diff-informed mode and adds location override to select call locations instead of sink locations |
ThreadUnsafeICryptoTransformLambda.ql | Enables diff-informed mode for parallel crypto transform usage detection |
UnsafeDeserializationQuery.qll | Enables diff-informed mode for primary configs, disables for secondary configs used in unsafe deserialization queries |
ExternalAPIsQuery.qll | Enables diff-informed mode for external API data flow tracking |
ConditionalBypassQuery.qll | Enables diff-informed mode and adds location override to select both sink and sensitive method call locations |
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.
LGTM under the conditions that
(1) DCA looks good
(2) The discussion started here is resolved.
@@ -78,6 +78,8 @@ private module RemoteSourceToExternalApiConfig implements DataFlow::ConfigSig { | |||
predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource } | |||
|
|||
predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode } | |||
|
|||
predicate observeDiffInformedIncrementalMode() { any() } |
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.
This will also affect the output of cs/count-untrusted-data-external-api
(if run in diff informed mode). However, this query is not included in any query suite, so this should be ok.
This PR enables diff-informed mode on queries that select a location other than dataflow source or sink. This entails adding a non-trivial location override that returns the locations that are actually selected.
Prior work includes PRs like #19663, #19759, and #19817. This PR uses the same patch script as those PRs to find candidate queries to convert to diff-enabled. This is the final step in mass-enabling diff-informed queries on all the languages.
Commit-by-commit reviewing is recommended.