Repository navigation
feat: decide advisory argument conditions at each call site - #119
Merged
Merged
Conversation
A call meeting an argument condition is reachable with the condition met; one passing another known value is ruled out, and the vulnerable requirement says which call and why; one passing a variable or unpacking arguments leaves it pending review. The same holds for attacker input, through a callee of another module, from the cache, and in advisory files, which reject a malformed position or default. The synthetic advisory's mode condition, whose text says the vulnerable mode is the default, now says so in its fields.
Call sites record what each argument denotes - a symbol, a short constant as Python writes it, or nothing known - once, in the call graph. External calls carry it through summaries and taint flows carry it for their sink call, so a flow through a callee of another module is decided at the sink. An argument condition gains a position and a default. A call meeting it is reported with conditions_met; a call passing another known value does not reach the vulnerability, and the requirement's finding lists it in ruled_out; a call passing a variable or unpacking arguments leaves it pending review. CallSite.arguments replaces the argument counts nothing read; the cache format is bumped. Closes #110
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 #110.
Why
Advisory entry points carry
argumentconditions (yaml.loadis affected by the FullLoader CVEs only whenLoaderis FullLoader; aiohttp's traversal only withfollow_symlinks=True), but until now every condition was listed as pending review:yaml.load(f, Loader=yaml.SafeLoader)was reported reachable for every FullLoader CVE.What
Arguments(coretrace_python.interprocedural): what each positional and keyword argument of a call denotes — a symbol's canonical name, a constant as Python writes it (True,None,'/static', bounded to 64 characters so log messages and queries stay out of the cache), orNonewhen unknown — plusunpackedfor*args/**kwargs. Computed once per call inCallGraphAnalysis;CallGraph.arguments_at()serves it.CallSite.arguments(it replaces the argument counts, which nothing read),ExternalCall.arguments(kept through callees' summaries, so the deep sink call's arguments survive),TaintFlow.sink_arguments.Condition.positionandCondition.default, dumped and loaded in advisory files; a non-integer position or non-boolean default is rejected at load.check_conditions(entry, arguments): each argument condition is met, contradicted (another known value, or absent withoutdefault) or undecided (a variable, unpacking). Semantic conditions are always pending.reachable-vulnerabilityand the correlation skip calls that contradict a condition; findings gainconditions_met, andconditions_pending_reviewlists only what is left.vulnerable-dependencylists the contradicting calls inruled_out(app:4 python.vulnlib.parse(mode=python.vulnlib.SAFE)) — the evidence that the imported requirement is not reached there — and now requires the call graph.Tests
tests/test_argument_conditions.py(19): met by keyword, by position and by default; a constant met with nothing left pending; contradicted by another symbol, positionally, byNone, byFalse, by absence — each ruled out with its reason and the requirement stayingimported; undecided by a variable,**options,*rest; exploitable only through the meeting call; a flow through a callee of another module decided at its sink; the same decisions from the cache; file round trip; malformedposition/defaultrejected; a long constant not recorded (checked to fail without the bound). The synthetic advisory oftest_advisory_evidence.pystates in its fields what its text already said (mode is the default, second argument); the cache codec test round-trips sites and external calls with arguments; the shipped manifest test gainsinterprocedural.callgraphforvulnerable-dependency.951 passed; ruff and mypy clean. Usage guide documents the notation,
position,default,conditions_metandruled_out.