Repository navigation
feat: let a safe argument take kinds off a sink call - #122
Merged
Merged
Conversation
Only a safe value given explicitly, by keyword or at its position, takes the model's kinds off the sink, directly or through a callee, and only those kinds; an absent argument, another value, a variable or unpacked arguments leave it. yaml.load with SafeLoader, CSafeLoader, the yaml.loader spelling or CBaseLoader is no insecure deserialization; with no loader, Loader, FullLoader or UnsafeLoader it still is. The plugin guide must document the model.
yaml.load(data, Loader=yaml.SafeLoader) was reported as a critical insecure deserialization. A SafeArgument model names the argument, by keyword or position, and the values that make a call to the sink safe for some kinds; the taint engine applies it where it reports a sink, directly and through a callee's summary. The reading of a call's arguments moves to Arguments.given, shared with the advisory conditions. Shipped for yaml.load with SafeLoader, BaseLoader and their C versions under every spelling. Closes #121
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 #121.
Why
yaml.load(data, Loader=yaml.SafeLoader)— the documented safe way to load YAML — was reported as a criticalinsecure-deserialization: theyaml.loadsink ignored its arguments.What
SafeArgument(symbol, argument, values, position=None, kinds=ALL)(coretrace_python.taint): a call to the sink whose argument, by keyword or atposition, denotes one ofvaluesis not a sink forkinds. Values are written as call sites record them (Advisories: check argument conditions at the call site #110): a symbol by canonical name, a constant as Python writes it._TaintProblem.guarded()where the taint engine reports a sink, directly and through a callee's summary; it takes off only the model's kinds (a sink with other kinds stays for them).Arguments.given(keyword, position): the one reading of a call's arguments, moved out of the advisory correlation, which now uses it too.yaml.load:SafeLoader,BaseLoader,CSafeLoader,CBaseLoader, each under its two spellings (yaml.Xandyaml.loader.X/yaml.cyaml.X), forDESERIALIZATIONonly — the advisory conditions decide theADVISORYkind on their own.repr(models)).Tests
tests/test_safe_arguments.py(17): the table accessor; safe by keyword and position vs absent, another value, a variable and**options; safe through a callee; only the model's kinds taken off; end to end,yaml.load(request.data, …)withSafeLoader,CSafeLoader,yaml.loader.SafeLoaderandyaml.cyaml.CBaseLoaderis no finding, while no loader,yaml.Loader,FullLoaderandUnsafeLoaderstill are.test_docs.pynow requires the model in the plugin guide.968 passed; ruff and mypy clean. No regression-corpus project passes a safe loader, so snapshots should not move (the CI regression job checks it).