diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatchPointsTo.qll b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatchPointsTo.qll index 6e5f80c9e98..f87631372d8 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatchPointsTo.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/DataFlowDispatchPointsTo.qll @@ -636,16 +636,22 @@ class SpecialCall extends DataFlowSourceCall, TSpecialCall { } } -/** A call to a summarized callable. */ +/** + * A call to a summarized callable, a `LibraryCallable`. + * + * This is a potential call, really. It has an empty charpred, so any `CallNode` is allowed. + * It is here to stake out territory, as otherwise a call to `map` would be considered a `ClassCall` + * and not be available for a summary. + */ class LibraryCall extends NormalCall { - LibraryCallable callable; - - LibraryCall() { call = callable.getACall() } - // TODO: Implement Python calling convention? override Node getArg(int n) { result = TCfgNode(call.getArg(n)) } - override DataFlowCallable getCallable() { result.asLibraryCallable() = callable } + // We cannot refer to a `LibraryCallable` here, + // as that could in turn refer to type tracking. + // This call will be tied to a `LibraryCallable` via + // `getViableCallabe` when the global data flow is assembled. + override DataFlowCallable getCallable() { none() } } /** @@ -747,7 +753,18 @@ private class SummaryPostUpdateNode extends SummaryNode, PostUpdateNode { } /** Gets a viable run-time target for the call `call`. */ -DataFlowCallable viableCallable(DataFlowSourceCall call) { result = call.getCallable() } +DataFlowCallable viableCallable(DataFlowSourceCall call) { + result = call.getCallable() + or + // A call to a library callable with a flow summary + // In this situation we can not resolve the callable from the call, + // as that would make data flow depend on type tracking. + // Instead we reolve the call from the summary. + exists(LibraryCallable callable | + result = TLibraryCallable(callable) and + call.getNode() = callable.getACall() + ) +} private newtype TReturnKind = TNormalReturnKind() diff --git a/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll b/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll index 8cba990744f..a4f71cf4eee 100644 --- a/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll +++ b/python/ql/lib/semmle/python/dataflow/new/internal/TypeTrackerSpecific.qll @@ -43,7 +43,13 @@ private DataFlowPrivate::DataFlowCallable getCallableForArgument( ) } -/** Holds if `nodeFrom` steps to `nodeTo` by being passed as a parameter in a call. */ +/** + * Holds if `nodeFrom` steps to `nodeTo` by being passed as a parameter in a call. + * + * Flow into summarized library methods is not included, as that will lead to negative + * recursion (or, at best, terrible performance), since identifying calls to library + * methods is done using API graphs (which uses type tracking). + */ predicate callStep(DataFlowPublic::ArgumentNode nodeFrom, DataFlowPrivate::ParameterNodeImpl nodeTo) { // TODO: Support special methods? exists(DataFlowPrivate::DataFlowCallable callable, int i | diff --git a/python/ql/test/experimental/dataflow/summaries/TestSummaries.qll b/python/ql/test/experimental/dataflow/summaries/TestSummaries.qll index 3bb556a3a8e..f9034c301ef 100644 --- a/python/ql/test/experimental/dataflow/summaries/TestSummaries.qll +++ b/python/ql/test/experimental/dataflow/summaries/TestSummaries.qll @@ -2,6 +2,25 @@ private import python private import semmle.python.dataflow.new.FlowSummary private import semmle.python.ApiGraphs +/** This module ensures that the `callStep` predicate in + * our type tracker implelemtation does not refer to the + * `getACall` predicate on `SummarizedCallable`. + */ +module RecursionGuard { + private import semmle.python.dataflow.new.internal.TypeTrackerSpecific as TT + + private class RecursionGuard extends SummarizedCallable { + RecursionGuard() { this = "RecursionGuard" } + + override CallNode getACall() { + result.getFunction().(NameNode).getId() = this and + (TT::callStep(_, _) implies any()) + } + + override DataFlow::ArgumentNode getACallback() { result.asExpr().(Name).getId() = this } + } +} + private class SummarizedCallableIdentity extends SummarizedCallable { SummarizedCallableIdentity() { this = "identity" }