Python: Modelling of the Standard Library #16840
Conversation
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.
|
Sorry, I somehow pushed to the wrong branch. The force-push should just reset the world without any change.. |
|
I have set no change note. I suppose a change note will be added when this branch is merged into main. |
There was a problem hiding this comment.
Overall looks OK to me, but it would have been nice to have some tests highlighting why flow through some of the more non-obvious methods are good. Value-flow for copy.copy makes 100% sense, but things like taint-flow for subprocess.Popen simply didn't make sense to me 🤷
I assume bc55117 was caused by some code using keywords which we didn't model previously?
| this = | ||
| DD::selfTracker(subclassRef() | ||
| .getAValueReachableFromSource() | ||
| .asExpr() | ||
| .(ClassExpr) | ||
| .getInnerScope()) | ||
| or |
There was a problem hiding this comment.
could you add a test indicating we can now do this?
There was a problem hiding this comment.
Ah good point...this is one bit I am somewhat unsure of, do we want this? Also, it should trigger a performance check.
There was a problem hiding this comment.
I see you added this in 77a0087 -- I think a DCA performance check for this whole PR is in place yes 👍
There was a problem hiding this comment.
I have set off a standard nightly/nightly DCA run. I did not include any extra options, so it should still extract the standard library; thus we should see a slight slowdown from the new modelling and we can gauge that it is not huge. (If we did stop extraction, that speedup would drown out what we are looking for.)
Co-authored-by: Rasmus Wriedt Larsen <rasmuswriedtlarsen@gmail.com>
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
|
DCA run looks fine. |
8864ea3
into
github:yoff-python-stop-extracting-std-lib
These are the models gathered so far during experiments not extracting the standard library by default.
A good part of the models are generated by special automation. Where possible, I have made manual adjustments to make them align with their documentation and not be overly specific.
Commit-by-commit review should be beneficiel.