Skip to content
Merged
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
19 changes: 17 additions & 2 deletions python/ql/src/semmle/python/dataflow/new/TypeTracker.qll
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,26 @@ private predicate typePreservingStep(Node nodeFrom, Node nodeTo) {
nodeFrom = nodeTo.(PostUpdateNode).getPreUpdateNode()
}

/**
* Gets a callable for the call where `nodeFrom` is used as the `i`'th argument.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Gets a callable for the call where `nodeFrom` is used as the `i`'th argument.
* Gets the callable for the call where `nodeFrom` is used as the `i`'th argument.

I hope we only have one callable for each call, otherwise I think we confuse the data-flow library.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I wasn't sure, and went with the safer option.

Copy link
Member Author

@RasmusWL RasmusWL Nov 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I actually tried this out, and it turns out that it can have multiple results. See commit ab856d6.

I'm not sure whether this will be confusing for the data-flow library, since we're in TypeTracker land for these predicates 😊

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, of course, I did feel that I had not been thinking long enough about this. This must be ok, since we cannot always know which callable to choose. I think it will also be alleviated by CFG splitting, but in general it will have to be ok.

*
* Helper predicate to avoid bad join order experienced in `callStep`.
* This happened when `isParameterOf` was joined _before_ `getCallable`.
*/
pragma[nomagic]
private DataFlowCallable getCallableForArgument(ArgumentNode nodeFrom, int i) {
exists(DataFlowCall call |
nodeFrom.argumentOf(call, i) and
result = call.getCallable()
)
}

/** Holds if `nodeFrom` steps to `nodeTo` by being passed as a parameter in a call. */
predicate callStep(ArgumentNode nodeFrom, ParameterNode nodeTo) {
// TODO: Support special methods?
exists(DataFlowCall call, int i |
nodeFrom.argumentOf(call, i) and nodeTo.isParameterOf(call.getCallable(), i)
exists(DataFlowCallable callable, int i |
callable = getCallableForArgument(nodeFrom, i) and
nodeTo.isParameterOf(callable, i)
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
def foo(foo_x): # $tracked
print("foo", foo_x) # $tracked


def bar(bar_x): # $tracked
print("bar", bar_x) # $tracked


if len(__file__) % 2 == 0:
f = foo
else:
f = bar

x = tracked # $tracked
f(x) # $tracked