55d822cc56
Splits `ModuleVariableNode` away from `LocalSourceNode`, instead creating a class `TypeTrackingNode` that encapsulates both of these. This means we no longer have module variable nodes as part of `LocalSourceNode` (which is good, since they have no "local" aspect to them), and hence we can have `LocalSourceNode` inherit directly from `ExprNode` (which makes the API a bit nicer). Unfortunately these are breaking changes, so we can't actually fulfil the above two desiderata until the `track` and `backtrack` methods on `LocalSourceNode` have been fully deprecated. For this reason, we preserve the present implementation of `LocalSourceNode`, and instead lay the foundation for switching over in the future, by deprecating `track` and `backtrack` on `LocalSourceNode`.
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 :|