Python: Container summaries, part 1 #13146
Conversation
yoff
force-pushed
the
python/container-summaries-1
branch
3 times, most recently
from
5b96644 to
103a0d9
Compare
yoff
force-pushed
the
python/container-summaries-1
branch
from
f843551 to
d908241
Compare
Also: - turn on flow summaries for taint - do not restrict node type (as now we need summary nodes)
yoff
force-pushed
the
python/container-summaries-1
branch
2 times, most recently
from
5d68473 to
145eaf3
Compare
|
I am thinking to not add a change note yet, but add one when all the container steps are merged...happy to hear other suggestions, though. |
| @@ -159,7 +162,7 @@ predicate stringManipulation(DataFlow::CfgNode nodeFrom, DataFlow::CfgNode nodeT | |||
| * is currently very imprecise, as an example, since we model `dict.get`, we treat any | |||
| * `<tainted object>.get(<arg>)` will be tainted, whether it's true or not. | |||
| */ | |||
| predicate containerStep(DataFlow::CfgNode nodeFrom, DataFlow::Node nodeTo) { | |||
| predicate containerStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) { | |||
There was a problem hiding this comment.
NIT: I don't think this change was required, but it doesn't matter 🤷
There was a problem hiding this comment.
In fact it matters greatly, since some flow steps will now involve nodes generated by flow summaries. It may not be visible until more summaries are added and we look at load and store steps, but in the branch with all the summaries, it makes a huge difference :-)
| or | ||
| FlowSummaryImpl::Private::Steps::summaryLocalStep(nodeFrom, nodeTo, false) |
There was a problem hiding this comment.
I assume this is also done by other languages that utilize flow-summaries?
There was a problem hiding this comment.
Yes, and in hindsight, it should have been added with the summary work; I think I just overlooked it back then..
In the first commit we remove the explicit steps, in the second we add flow summaries. Viewing test file changes for all commits shows the end result of the swap, while viewing changes for individual commits reveals the effect of removing the explicit steps.
In the third commit, we remove nodes with no location from the basic dataflow and taint flow tests, as they do not really scale with many flow summaries.
The CI validating that tests pass on all commits, can be seen in the Commits tab.