Python: revert "remove imprecise container steps" for performance reasons#22098
Open
owen-mc wants to merge 3 commits into
Open
Python: revert "remove imprecise container steps" for performance reasons#22098owen-mc wants to merge 3 commits into
owen-mc wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adjusts Python’s dataflow/taint-tracking internals to revert a prior change around “imprecise container steps,” aiming to improve performance by changing how container-related flow is represented and by updating affected models and tests accordingly.
Changes:
- Disables default implicit taint reads and reshapes container-related taint propagation to avoid expensive/precise content expansion.
- Refactors internal content/content-set handling and flow-summary plumbing to work with the revised content representation.
- Updates/realigns a large set of Python security and library tests (including expected path outputs and inline expectations) with the new behavior, and removes a couple of YAML summary models.
Show a summary per file
| File | Description |
|---|---|
| python/ql/test/query-tests/Security/CWE-943-NoSqlInjection/NoSqlInjection.expected | Updated expected path graph output to match revised container/content flow behavior. |
| python/ql/test/query-tests/Security/CWE-918-ServerSideRequestForgery/PartialServerSideRequestForgery.expected | Updated expected path graph output after container/content flow adjustments. |
| python/ql/test/query-tests/Security/CWE-312-CleartextStorage/CleartextStorage.expected | Updated expected path graph output for revised container/content handling. |
| python/ql/test/query-tests/Security/CWE-312-CleartextLogging/test.py | Updated inline expectations to record newly spurious source/alert behavior. |
| python/ql/test/query-tests/Security/CWE-312-CleartextLogging/CleartextLogging.expected | Updated expected results/paths to reflect new (less precise) container-related flow outcomes. |
| python/ql/test/query-tests/Security/CWE-209-StackTraceExposure/StackTraceExposure.expected | Updated expected path graph output following content/implicit-read changes. |
| python/ql/test/query-tests/Security/CWE-079-ReflectedXss/ReflectedXss.expected | Updated expected edges/nodes due to revised container/content steps. |
| python/ql/test/query-tests/Security/CWE-078-UnsafeShellCommandConstruction/UnsafeShellCommandConstruction.expected | Updated expected path graph output with new container modeling behavior. |
| python/ql/test/query-tests/Security/CWE-020-ExternalAPIs/UntrustedDataToExternalAPI.expected | Updated expected path graph output after container/content changes. |
| python/ql/test/query-tests/Security/CVE-2018-1281/BindToAllInterfaces.expected | Updated expected flows/paths to match new container/content representation. |
| python/ql/test/library-tests/frameworks/tornado/taint_test.py | Adjusted taint expectations for container-returning APIs under the new propagation rules. |
| python/ql/test/library-tests/frameworks/stdlib/test_re.py | Updated taint expectations around re APIs to match revised object-vs-contained-value taint behavior. |
| python/ql/test/library-tests/frameworks/gradio/taint_step_test.expected | Updated expected flow edges/nodes to align with new container/content behavior. |
| python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep/test_unpacking.py | Annotated a now-spurious taint expectation after the behavior change. |
| python/ql/test/library-tests/dataflow/tainttracking/defaultAdditionalTaintStep/test_collections.py | Adjusted construction/access expectations (including a documented missing taint case). |
| python/ql/test/library-tests/dataflow/summaries/summaries.expected | Updated expected summary-related flows for revised container/content semantics. |
| python/ql/test/library-tests/dataflow/sensitive-data/test.py | Updated commentary/expectations to reflect container cross-talk behavior. |
| python/ql/test/experimental/query-tests/Security/CWE-1427-PromptInjection/PromptInjection.expected | Updated expected path graph output after container/content changes. |
| python/ql/test/experimental/query-tests/Security/CWE-091-XsltInjection/XsltInjection.expected | Updated expected path graph output after container/content changes. |
| python/ql/test/experimental/query-tests/Security/CWE-074-RemoteCommandExecution/RemoteCommandExecution.expected | Updated expected path graph output after container/content changes. |
| python/ql/test/experimental/query-tests/Security/CWE-022-UnsafeUnpacking/UnsafeUnpack.expected | Updated expected path graph output after container/content changes. |
| python/ql/test/experimental/query-tests/Security/CWE-022-TarSlip/TarSlip.expected | Updated expected path graph output after container/content changes. |
| python/ql/src/Variables/LoopVariableCapture/LoopVariableCaptureQuery.qll | Adjusted implicit-read policy for this config to match new container/content semantics. |
| python/ql/lib/semmle/python/frameworks/xml.model.yml | Removed a summary-model entry affected by the new container/content behavior. |
| python/ql/lib/semmle/python/frameworks/Stdlib.qll | Updated/removed summaries and adjusted behavior around iterable/string conversion handling (including join). |
| python/ql/lib/semmle/python/frameworks/lxml.model.yml | Removed a summary-model entry affected by the new container/content behavior. |
| python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackingImpl.qll | Updated type-tracking steps to use the revised content representation. |
| python/ql/lib/semmle/python/dataflow/new/internal/TaintTrackingPrivate.qll | Removed default implicit reads and reworked container-step logic; added join handling in string manipulation. |
| python/ql/lib/semmle/python/dataflow/new/internal/FlowSummaryImpl.qll | Updated content encoding/summary component construction to align with new content representation. |
| python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPublic.qll | Refactored ContentSet representation and related APIs to match the new approach. |
| python/ql/lib/semmle/python/dataflow/new/internal/DataFlowPrivate.qll | Updated store/read/clear step predicates to use the revised content/content-set representation and removed conversion read steps. |
| python/ql/consistency-queries/DataFlowConsistency.ql | Updated consistency exclusions to match removal of conversion read steps. |
Review details
- Files reviewed: 32/32 changed files
- Comments generated: 1
- Review effort level: Low
Comment on lines
+164
to
+167
| * Holds if taint can flow from `nodeFrom` to `nodeTo` with a step related to containers | ||
| * (lists/sets/dictionaries): literals, constructor invocation, methods. Note that this | ||
| * is currently very imprecise, as an example, since we model `dict.get`, we treat any | ||
| * `<tainted object>.get(<arg>)` will be tainted, whether it's true or not. |
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.
No description provided.