Repository navigation
feat: taint the keys of a **mapping as the keyword names of the call - #196
Merged
Merged
Conversation
Add the keys heap field and key taint for dict literals, dict(...), comprehensions and subscript assignment; report the keys of a ** argument as the (None, "**") way of an advisory flow; record Arguments.keyword_unpacked and fold a ** literal with constant keys that nothing else uses into the keywords (cache format 12); decide the new keyword_name advisory condition from them. Closes #193.
Merged
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.
Closes #193.
Django's ORM takes column aliases and lookups as keyword names; five advisories are SQL injections through those names when a project expands an attacker-controlled mapping. The engine reported an argument passed through
**with neither position nor keyword, right for the mapping's values, and had no notion that its keys become keyword names, so a curated entry point could not say "reached when the attacker chooses a keyword name", and a project expanding{request.GET["sort"]: F("id")}looked like one expanding a constant mapping.keys, besideelementsandattributes, filled by the taint engine only (the points-to solver never does) for a dict literal,dict(...)(a copied mapping,dict(**m),dict([(k, v)])), a comprehension (already lowered to a subscript assignment) andd[k] = v;{**d}copies it. It is not part of the deep taint, so no existing flow changes.f(**m)with tainted keys adds(None, "**")(KEYWORD_NAMES) to the flow'spassed_as, for advisory sinks only: no regular sink reads a keyword name.Arguments.keyword_unpackedsays a call expands a mapping whose keys it does not write; a**literal with constant string keys that nothing else uses is folded intokeywords, sogiven(), argument conditions and safe arguments decide it (more precise than before). Cache format 12.keyword_name(text only): met on the(None, "**")way, pending when the call expands a mapping whose keys are not established (a parameter, a mapping built elsewhere,dict(zip(...)), a literal used elsewhere), contradicted by a call writing every keyword name, whichruled_outreports asannotate(keywords=total). With the issue's reproduction and Django 5.2.5 pinned,listingis exploitable with the condition met andfixedalone leaves the advisorynot_affected.One choice beyond the design:
_keyword_entriesonly folds a literal the call alone uses, the guardsummaries._literal_entriesalready has; without itm = {}; m[alias] = F("id"); annotate(**m)would be ruled out at reachability while the taint stage finds it exploitable. A literal used elsewhere is therefore pending rather than contradicted.Documented limits: keys through a project wrapper stay pending;
update,setdefault,keys()and iteration carry no key taint; aPhi-bound mapping is unknown. Curated entry points namingQuerySet.annotatedo not yet matchModel.objects.annotate(derived as a project symbol): a separate issue.Tests:
tests/test_keyword_name_conditions.py(20 cases) andtests/test_key_taint.py(26), failing before the change;pytest -m "not regression"1581 passed, 2 skipped; regression corpus 19 passed under Python 3.12, snapshots unchanged;ruff check .andmypyclean.