diff --git a/python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql b/python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql index dc5633863d2..069f255f198 100644 --- a/python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql +++ b/python/ql/src/Security/CVE-2018-1281/BindToAllInterfaces.ql @@ -32,21 +32,7 @@ private DataFlow::LocalSourceNode vulnerableHostnameRef(DataFlow::TypeTracker t, result.asExpr() = allInterfacesStrConst ) or - // Due to bad performance when using normal setup with `vulnerableHostnameRef(t2, hostname).track(t2, t)` - // we have inlined that code and forced a join - exists(DataFlow::TypeTracker t2 | - exists(DataFlow::StepSummary summary | - vulnerableHostnameRef_first_join(t2, hostname, result, summary) and - t = t2.append(summary) - ) - ) -} - -pragma[nomagic] -private predicate vulnerableHostnameRef_first_join( - DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary -) { - DataFlow::StepSummary::step(vulnerableHostnameRef(t2, hostname), res, summary) + exists(DataFlow::TypeTracker t2 | result = vulnerableHostnameRef(t2, hostname).track(t2, t)) } /** Gets a reference to a hostname that can be used to bind to all interfaces. */ @@ -59,21 +45,7 @@ private DataFlow::LocalSourceNode vulnerableAddressTuple(DataFlow::TypeTracker t t.start() and result.asExpr() = any(Tuple tup | tup.getElt(0) = vulnerableHostnameRef(hostname).asExpr()) or - // Due to bad performance when using normal setup with `vulnerableAddressTuple(t2, hostname).track(t2, t)` - // we have inlined that code and forced a join - exists(DataFlow::TypeTracker t2 | - exists(DataFlow::StepSummary summary | - vulnerableAddressTuple_first_join(t2, hostname, result, summary) and - t = t2.append(summary) - ) - ) -} - -pragma[nomagic] -private predicate vulnerableAddressTuple_first_join( - DataFlow::TypeTracker t2, string hostname, DataFlow::Node res, DataFlow::StepSummary summary -) { - DataFlow::StepSummary::step(vulnerableAddressTuple(t2, hostname), res, summary) + exists(DataFlow::TypeTracker t2 | result = vulnerableAddressTuple(t2, hostname).track(t2, t)) } /** Gets a reference to a tuple for which the first element is a hostname that can be used to bind to all interfaces. */ diff --git a/python/ql/src/semmle/python/ApiGraphs.qll b/python/ql/src/semmle/python/ApiGraphs.qll index 5f1fbbb9626..48f30308170 100644 --- a/python/ql/src/semmle/python/ApiGraphs.qll +++ b/python/ql/src/semmle/python/ApiGraphs.qll @@ -433,18 +433,7 @@ module API { use(_, src) and result = src or - // Due to bad performance when using `trackUseNode(t2, attr_name).track(t2, t)` - // we have inlined that code and forced a join - exists(DataFlow::StepSummary summary | - t = trackUseNode_first_join(src, result, summary).append(summary) - ) - } - - pragma[nomagic] - private DataFlow::TypeTracker trackUseNode_first_join( - DataFlow::LocalSourceNode src, DataFlow::LocalSourceNode res, DataFlow::StepSummary summary - ) { - DataFlow::StepSummary::step(trackUseNode(src, result), res, summary) + exists(DataFlow::TypeTracker t2 | result = trackUseNode(src, t2).track(t2, t)) } cached diff --git a/python/ql/src/semmle/python/Concepts.qll b/python/ql/src/semmle/python/Concepts.qll index d8e65cb2e09..4be00ba5bf9 100644 --- a/python/ql/src/semmle/python/Concepts.qll +++ b/python/ql/src/semmle/python/Concepts.qll @@ -570,21 +570,7 @@ module Cryptography { arg = any(KeyGeneration::Range r).getKeySizeArg() and result = arg.getALocalSource() or - // Due to bad performance when using normal setup with we have inlined that code and forced a join - exists(DataFlow::TypeBackTracker t2 | - exists(DataFlow::StepSummary summary | - keysizeBacktracker_first_join(t2, arg, result, summary) and - t = t2.prepend(summary) - ) - ) - } - - pragma[nomagic] - private predicate keysizeBacktracker_first_join( - DataFlow::TypeBackTracker t2, DataFlow::Node arg, DataFlow::Node res, - DataFlow::StepSummary summary - ) { - DataFlow::StepSummary::step(res, keysizeBacktracker(t2, arg), summary) + exists(DataFlow::TypeBackTracker t2 | result = keysizeBacktracker(t2, arg).backtrack(t2, t)) } /** Gets a back-reference to the keysize argument `arg` that was used to generate a new key-pair. */ diff --git a/python/ql/src/semmle/python/dataflow/new/TypeTracker.qll b/python/ql/src/semmle/python/dataflow/new/TypeTracker.qll index d4ce5f606ac..bba7ecf367b 100644 --- a/python/ql/src/semmle/python/dataflow/new/TypeTracker.qll +++ b/python/ql/src/semmle/python/dataflow/new/TypeTracker.qll @@ -51,8 +51,8 @@ module StepSummary { * heap and/or inter-procedural step from `nodeFrom` to `nodeTo`. */ cached - predicate step(LocalSourceNode nodeFrom, Node nodeTo, StepSummary summary) { - exists(Node mid | typePreservingStep*(nodeFrom, mid) and smallstep(mid, nodeTo, summary)) + predicate step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo, StepSummary summary) { + exists(Node mid | nodeFrom.flowsTo(mid) and smallstep(mid, nodeTo, summary)) } /** @@ -63,7 +63,7 @@ module StepSummary { * type-preserving steps. */ predicate smallstep(Node nodeFrom, Node nodeTo, StepSummary summary) { - typePreservingStep(nodeFrom, nodeTo) and + jumpStep(nodeFrom, nodeTo) and summary = LevelStep() or callStep(nodeFrom, nodeTo) and summary = CallStep() @@ -80,12 +80,6 @@ module StepSummary { } } -/** Holds if it's reasonable to expect the data flow step from `nodeFrom` to `nodeTo` to preserve types. */ -private predicate typePreservingStep(Node nodeFrom, Node nodeTo) { - simpleLocalFlowStep(nodeFrom, nodeTo) or - jumpStep(nodeFrom, nodeTo) -} - /** * Gets a callable for the call where `nodeFrom` is used as the `i`'th argument. * @@ -274,10 +268,10 @@ class TypeTracker extends TTypeTracker { * heap and/or inter-procedural step from `nodeFrom` to `nodeTo`. */ pragma[inline] - TypeTracker step(LocalSourceNode nodeFrom, Node nodeTo) { + TypeTracker step(LocalSourceNode nodeFrom, LocalSourceNode nodeTo) { exists(StepSummary summary | - StepSummary::step(nodeFrom, nodeTo, summary) and - result = this.append(summary) + StepSummary::step(nodeFrom, pragma[only_bind_out](nodeTo), pragma[only_bind_into](summary)) and + result = this.append(pragma[only_bind_into](summary)) ) } @@ -312,7 +306,7 @@ class TypeTracker extends TTypeTracker { result = this.append(summary) ) or - typePreservingStep(nodeFrom, nodeTo) and + simpleLocalFlowStep(nodeFrom, nodeTo) and result = this } } @@ -453,7 +447,7 @@ class TypeBackTracker extends TTypeBackTracker { this = result.prepend(summary) ) or - typePreservingStep(nodeFrom, nodeTo) and + simpleLocalFlowStep(nodeFrom, nodeTo) and this = result } } diff --git a/python/ql/src/semmle/python/dataflow/new/internal/LocalSources.qll b/python/ql/src/semmle/python/dataflow/new/internal/LocalSources.qll index 7aa311df76b..134507add94 100644 --- a/python/ql/src/semmle/python/dataflow/new/internal/LocalSources.qll +++ b/python/ql/src/semmle/python/dataflow/new/internal/LocalSources.qll @@ -41,7 +41,6 @@ class LocalSourceNode extends Node { cached LocalSourceNode() { not comes_from_cfgnode(this) and - not this instanceof ModuleVariableNode and // Currently, we create synthetic post-update nodes for // - arguments to calls that may modify said argument // - direct reads a writes of object attributes diff --git a/python/ql/src/semmle/python/frameworks/Cryptography.qll b/python/ql/src/semmle/python/frameworks/Cryptography.qll index 3396d7dfa55..266d41897d3 100644 --- a/python/ql/src/semmle/python/frameworks/Cryptography.qll +++ b/python/ql/src/semmle/python/frameworks/Cryptography.qll @@ -83,23 +83,11 @@ private module CryptographyModel { result.(DataFlow::CallCfgNode).getFunction() = curveClassWithKeySize(keySize) and origin = result or - // Due to bad performance when using normal setup with we have inlined that code and forced a join exists(DataFlow::TypeTracker t2 | - exists(DataFlow::StepSummary summary | - curveClassInstanceWithKeySize_first_join(t2, keySize, origin, result, summary) and - t = t2.append(summary) - ) + result = curveClassInstanceWithKeySize(t2, keySize, origin).track(t2, t) ) } - pragma[nomagic] - private predicate curveClassInstanceWithKeySize_first_join( - DataFlow::TypeTracker t2, int keySize, DataFlow::Node origin, DataFlow::Node res, - DataFlow::StepSummary summary - ) { - DataFlow::StepSummary::step(curveClassInstanceWithKeySize(t2, keySize, origin), res, summary) - } - /** Gets a reference to a predefined curve class instance with a specific key size (in bits), as well as the origin of the class. */ DataFlow::Node curveClassInstanceWithKeySize(int keySize, DataFlow::Node origin) { curveClassInstanceWithKeySize(DataFlow::TypeTracker::end(), keySize, origin).flowsTo(result) diff --git a/python/ql/src/semmle/python/regex.qll b/python/ql/src/semmle/python/regex.qll index f558669ce32..365c36d0d2f 100644 --- a/python/ql/src/semmle/python/regex.qll +++ b/python/ql/src/semmle/python/regex.qll @@ -82,21 +82,7 @@ private DataFlow::LocalSourceNode re_flag_tracker(string flag_name, DataFlow::Ty result.asCfgNode() = binop ) or - // Due to bad performance when using normal setup with `re_flag_tracker(t2, attr_name).track(t2, t)` - // we have inlined that code and forced a join - exists(DataFlow::TypeTracker t2 | - exists(DataFlow::StepSummary summary | - re_flag_tracker_first_join(t2, flag_name, result, summary) and - t = t2.append(summary) - ) - ) -} - -pragma[nomagic] -private predicate re_flag_tracker_first_join( - DataFlow::TypeTracker t2, string flag_name, DataFlow::Node res, DataFlow::StepSummary summary -) { - DataFlow::StepSummary::step(re_flag_tracker(flag_name, t2), res, summary) + exists(DataFlow::TypeTracker t2 | result = re_flag_tracker(flag_name, t2).track(t2, t)) } /** diff --git a/python/ql/test/experimental/dataflow/ApiGraphs/use.ql b/python/ql/test/experimental/dataflow/ApiGraphs/use.ql index f02bb048c58..2bcc05d443b 100644 --- a/python/ql/test/experimental/dataflow/ApiGraphs/use.ql +++ b/python/ql/test/experimental/dataflow/ApiGraphs/use.ql @@ -9,7 +9,11 @@ class ApiUseTest extends InlineExpectationsTest { override string getARelevantTag() { result = "use" } private predicate relevant_node(API::Node a, DataFlow::Node n, Location l) { - n = a.getAUse() and l = n.getLocation() + n = a.getAUse() and + l = n.getLocation() and + // Module variable nodes have no suitable location, so it's best to simply exclude them entirely + // from the inline tests. + not n instanceof DataFlow::ModuleVariableNode } override predicate hasActualResult(Location location, string element, string tag, string value) { diff --git a/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected b/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected index 82aa6e99c3c..adfc8c5a379 100644 --- a/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected +++ b/python/ql/test/experimental/dataflow/typetracking/moduleattr.expected @@ -1,6 +1,7 @@ module_tracker | import_as_attr.py:1:6:1:11 | ControlFlowNode for ImportExpr | module_attr_tracker +| import_as_attr.py:0:0:0:0 | ModuleVariableNode for Global Variable attr_ref in Module import_as_attr | | import_as_attr.py:1:20:1:35 | ControlFlowNode for ImportMember | | import_as_attr.py:1:28:1:35 | GSSA Variable attr_ref | | import_as_attr.py:3:1:3:1 | GSSA Variable x |