Python: Several standard library models #17454
Merged
+213
−20
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...
yoff
force-pushed
the
stdlib-optparse
branch
from
e0cec32
to
3434c38
Compare
yoff
changed the title
RasmusWL
requested changes
Oct 1, 2024
There was a problem hiding this comment.
I think I already approved all these changes as part of other chains of PRs, right?
In that case, I only have one nitpick: With this PR we have two files for modeling stdlib with MaD (https://github.com/github/codeql/blob/main/python/ql/lib/semmle/python/frameworks/Stdlib.model.yml) so we should consolidate to only using one location. Personally I feel the obvious place to put modeling of a new library would be ql/lib/semmle/python/frameworks/<package>.model.yml (such as the one for Asyncpg). So unless you strongly object, would you mind moving the new models to python/ql/lib/semmle/python/frameworks/Stdlib.model.yml? 🙏
Co-authored-by: Rasmus Wriedt Larsen <[email protected]>
Nice. I had totally missed that we had started using MaD for StdLib. In that case I will put the new models there too. I will also adjust #17565. |
tausbn
previously approved these changes
Oct 1, 2024
RasmusWL
reviewed
Oct 3, 2024
Co-authored-by: Rasmus Wriedt Larsen <[email protected]>
RasmusWL
approved these changes
Oct 3, 2024
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.
This PR is merging the long-lived branch containing the modelling work to stop extracting the standard library by default.
Pull Request checklist
All query authors
Internal query authors only
.ql,.qll, or.qhelpfiles. See the documentation (internal access required).