Python/DSVW repro #17635
Draft
Python/DSVW repro #17635
Conversation
This file contains 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
- empty models for now - `summaryModel` of `codeql/python-all` will be added to shortly.
- `quote` together with `re.compile` recover regex injection alerts on haiwen/seahub - `quote_plus` recovers the URL redirection alert on DemocracyClub/EveryElection - `unquote` recovers path injection alerts on `cloudera/hue` - it was tedious finding justifications for the rest..
There is already a model there so we add to that one. We did observe that this existing model was blocked by the external MaD model. This is concerning and needs to be cleared up.
Two of the generated summaries have been excluded:
- ["re", "Member[split]", "Argument[0,pattern:]", "ReturnValue", "taint"]
From the documentation, it is not clear why pattern should figure in the return value, as that is the part denoting split point and thus all those instances are filtered out.
From the implementation
Spit function: https://github.com/python/cpython/blob/3.12/Lib/re/__init__.py#L199
_compile function being called by split: https://github.com/python/cpython/blob/3.12/Lib/re/__init__.py#L280
We see that in case the pattern is already a compiled `Pattern`, it is returned directly from _compile and could thus be part of the return value from split. This is probably not possible to arrange for an attacker, and so an FP in practice.
- ["urllib2", "Member[unquote]", "Argument[0,string:]", "ReturnValue", "taint"]
urllib2 seems to be only in Python2 (e.g. https://docs.python.org/2.7/library/urllib2.html) and I cannot locate the function unquote.
Co-authored-by: Rasmus Wriedt Larsen <[email protected]>
It is not clear from the code how this could happen and I do not remember the path I saw, perhaps it was unreasonable.
It would be nice to simplify to a single sequence content type..
MaD row numbers in provenance column
This is MaD...
- add comprehension functions as `DataFlowCallable`s - add comprehension call as `DataFlowCall` - create capture argument node for comprehension calls
For a comprehension `[x for x in l] - `l` is now a legal argument (in DataFlowPublic) - `l` is the argument of the comprehension function (in DataFlowDispatch) - the parameter of the comprehension function is being read rather than `l` (in IterableUnpacking) Thus the read that used to cross callable boundaries is now split into a arg-param edge and a read from that param.
We used to use the CfgNode for the comprehension itself. In cases where that is also an argument, say ```python ",".join([x for x in l]) ``` that would be an argument to two different calls causing a dataflow consistency violation.
- add yield as a dataflow return - replace comprehension store step with a store step to the yield
- adjust scope of argument, the argument is outside the called function - add missing post-update nodes for the new arguments
We now have a new callable, yielding new enclosing callables
Using the comprehension store step meant that all comprehensions would receive taint. This because comprehension flow now goes via a callable, meaning they share the return node.
- also adjust test expectations in experimental
More doc is needed, but this should turn the tests green
Co-authored-by: Rasmus Wriedt Larsen <[email protected]>
…atch.qll Co-authored-by: Taus <[email protected]>
We had accidentally used precise content leadingto blowup
…b.com/yoff/codeql into python/DSVW-repro
… python/DSVW-repro
- API graph subscript operator to understand comprehensions - captureJumpStep to not require definig value to exist - stdlib modelling: finditer returns list of match objects - adjust taint output of finditer - adjust `ReMatchMethodsSummary.getACall`
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.
Pull Request checklist
All query authors
.qhelp. See the documentation in this repository.Internal query authors only
.ql,.qll, or.qhelpfiles. See the documentation (internal access required).