11c71fdd18
This commit removes SSA nodes from the data flow graph. Specifically, for a definition and use such as ```python x = expr y = x + 2 ``` we used to have flow from `expr` to an SSA variable representing x and from that SSA variable to the use of `x` in the definition of `y`. Now we instead have flow from `expr` to the control flow node for `x` at line 1 and from there to the control flow node for `x` at line 2. Specific changes: - `EssaNode` from the data flow layer no longer exists. - Several glue steps between `EssaNode`s and `CfgNode`s have been deleted. - Entry nodes are now admitted as `CfgNodes` in the data flow layer (they were filtered out before). - Entry nodes now have a new `toString` taking into account that the module name may be ambigous. - Some tests have been rewritten to accomodate the changes, but only `python/ql/test/experimental/dataflow/basic/maximalFlowsConfig.qll` should have semantic changes. - Comments have been updated - Test output has been updated, but apart from `python/ql/test/experimental/dataflow/basic/maximalFlows.expected` only `python/ql/test/experimental/dataflow/typetracking-summaries/summaries.py` should have a semantic change. This is a bonus fix, probably meaning that something was never connected up correctly.
This test illustrates that you need to be very careful when adding additional taint-steps or dataflow steps using TypeTracker.
The basic setup is that we're modeling the behavior of a (fictitious) external library class MyClass, and (fictitious) source of such an instance (the source function).
class MyClass:
def __init__(self, value):
self.value = value
def get_value(self):
return self.value
We want to extend our analysis to obj.get_value() is also tainted if obj is a tainted instance of MyClass.
The actual type-tracking is done in SharedCode.qll, but it's the way we use it that matters.
In NaiveModel.ql we add an additional taint step from an instance of MyClass to calls of the bound method get_value (that we have tracked). It provides us with the correct results, but the path explanations are not very useful, since we are now able to cross functions in one step.
In ProperModel.ql we split the additional taint step in two:
- from tracked
objthat is instance ofMyClass, toobj.get_valuebut only exactly where the attribute is accessed (by anAttrNode). This is important, since if we allowed<any tracked qualifier>.get_valuewe would again be able to cross functions in one step. - from tracked
get_valuebound method to calls of it, but only exactly where the call is (by aCallNode). for same reason as above.
Try running the queries in VS Code to see the difference
Possible improvements
Using AttrNode directly in the code here means there is no easy way to add getattr support too all such predicates. Not really sure how to handle this in a generalized way though :|