Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Python type tracking now follows values stored in instance attributes such as `self.attr` across instance methods on the same class. As a result, analysis is more likely to recognize user-defined objects that are stored on `self` and used later in other methods, which may produce additional results.
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
/** Holds if there is a level step from `nodeFrom` to `nodeTo`, which does not depend on the call graph. */
predicate levelStepNoCall(Node nodeFrom, LocalSourceNode nodeTo) {
TypeTrackerSummaryFlow::levelStepNoCall(nodeFrom, nodeTo)
or
localFieldStep(nodeFrom, nodeTo)
}

/**
Expand Down Expand Up @@ -317,6 +319,51 @@ module TypeTrackingInput implements Shared::TypeTrackingInput<Location> {
)
}

/**
* Holds if `ref` accesses attribute `attr` of `self`, where `self` is the first
* parameter of an instance method of `cls` (i.e. an access of the form `self.attr`).
*
* Static methods and class methods are excluded, since their first parameter is not a
* `self` instance reference.
*/
private predicate selfAttrRef(Class cls, string attr, DataFlowPublic::AttrRef ref) {
exists(Function method, Name selfUse |
method = cls.getAMethod() and
not DataFlowDispatch::isStaticmethod(method) and
not DataFlowDispatch::isClassmethod(method) and
selfUse.getVariable() = method.getArg(0).(Name).getVariable() and
ref.getObject().asCfgNode().getNode() = selfUse and
ref.mayHaveAttributeName(attr)
)
}

/**
* Holds if `nodeFrom` is written to attribute `self.attr` in some instance method of a
* class, and `nodeTo` reads attribute `self.attr` in some (possibly different) instance
* method of the same class.
*
* This models flow through instance attributes (`self.foo`): a value stored into
* `self.foo` in one method can be read from `self.foo` in another method. Type-tracking
* handles the store and read steps via `AttrWrite`/`AttrRead`, but on its own it cannot
* relate the `self` of the writing method to the `self` of the reading method. Following
* the approach used for Ruby and JavaScript, we model this directly as a level step from
* the written value to the read reference, for any pair of methods on the class (not
* just from `__init__`).
*
* This is an over-approximation: it is instance-insensitive (it does not distinguish
Comment thread
owen-mc marked this conversation as resolved.
* between different instances of the same class) and order-insensitive (it does not
* require the write to happen before the read), matching the precision of
* instance-attribute handling for Ruby and JavaScript.
*/
private predicate localFieldStep(Node nodeFrom, LocalSourceNode nodeTo) {
exists(Class cls, string attr, DataFlowPublic::AttrWrite write, DataFlowPublic::AttrRead read |
selfAttrRef(cls, attr, write) and
nodeFrom = write.getValue() and
selfAttrRef(cls, attr, read) and
nodeTo = read
)
}

/**
* Holds if data can flow from `node1` to `node2` in a way that discards call contexts.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
testFailures
debug_callableNotUnique
pointsTo_found_typeTracker_notFound
| code/class_attr_assign.py:10:9:10:27 | ControlFlowNode for Attribute() | my_func |
| code/class_attr_assign.py:11:9:11:25 | ControlFlowNode for Attribute() | my_func |
| code/class_attr_assign.py:26:9:26:25 | ControlFlowNode for Attribute() | DummyObject.method |
| code/class_super.py:50:1:50:6 | ControlFlowNode for Attribute() | outside_def |
| code/conditional_in_argument.py:18:5:18:11 | ControlFlowNode for Attribute() | X.bar |
| code/func_defined_outside_class.py:21:1:21:11 | ControlFlowNode for Attribute() | A.foo |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ def __init__(self, func):
self.direct_ref = my_func

def later(self):
self.indirect_ref() # $ pt=my_func MISSING: tt=my_func
self.direct_ref() # $ pt=my_func MISSING: tt=my_func
self.indirect_ref() # $ pt=my_func tt=my_func
self.direct_ref() # $ pt=my_func tt=my_func

foo = Foo(my_func) # $ tt=Foo.__init__
foo.later() # $ pt,tt=Foo.later
Expand All @@ -23,7 +23,7 @@ def __init__(self):
self.obj = DummyObject()

def later(self):
self.obj.method() # $ pt=DummyObject.method MISSING: tt=DummyObject.method
self.obj.method() # $ pt=DummyObject.method tt=DummyObject.method


bar = Bar(my_func) # $ tt=Bar.__init__
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,10 @@ def __init__(self): # $ tracked=foo
self.foo = tracked # $ tracked=foo tracked

def print_foo(self): # $ MISSING: tracked=foo
print(self.foo) # $ MISSING: tracked=foo tracked
print(self.foo) # $ tracked MISSING: tracked=foo

def possibly_uncalled_method(self): # $ MISSING: tracked=foo
print(self.foo) # $ MISSING: tracked=foo tracked
print(self.foo) # $ tracked MISSING: tracked=foo
Comment thread
owen-mc marked this conversation as resolved.

instance = MyClass2()
print(instance.foo) # $ MISSING: tracked=foo tracked
Expand Down
27 changes: 27 additions & 0 deletions python/ql/test/library-tests/frameworks/hdbcli/pep249.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,30 @@
cursor.executemany("some sql", (42,)) # $ getSql="some sql"

cursor.close()


# Connection stored in a class attribute (`self._conn`) and used in another method.
#
# This is detected because type tracking includes a level step modelling flow through
# instance attributes: a value written to `self._conn` in one method (here `__init__`) can
# be read back from `self._conn` (directly or via a getter) in any other method on the same
# class. This follows the same approach used for instance fields in Ruby and JavaScript.
class Database:
def __init__(self):
self._conn = dbapi.connect(address="hostname", port=300, user="username")

def get_connection(self):
return self._conn

def run_via_getter(self):
conn = self.get_connection()
cursor = conn.cursor()
cursor.execute("getter sql") # $ getSql="getter sql"

def run_direct(self):
self._conn.execute("direct sql") # $ getSql="direct sql"


db = Database()
db.run_via_getter()
db.run_direct()
Loading