JS: Refine speculative step definition

Author: asgerf

javascript/ql/lib/semmle/javascript/dataflow/internal/TaintTrackingPrivate.qll

@@ -105,11 +105,21 @@ predicate defaultImplicitTaintRead(DataFlow::Node node, ContentSet c) {
   not optionalStep(node, _, _)
 }
 
+private predicate isArgumentToResolvedCall(DataFlow::Node arg) {
+  exists(DataFlowCall c |
+    exists(viableCallable(c)) and
+    isArgumentNode(arg, c, _)
+  )
+}
+
 predicate speculativeTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
-  exists(DataFlow::CallNode call, DataFlowCall c |
-    not exists(viableCallable(c)) and
-    c.asOrdinaryCall() = call and
+  exists(DataFlow::CallNode call |
     node1 = call.getAnArgument() and
-    node2 = call
+    node2 = call and
+    // A given node can appear as argument in more than one call. For example `x` in `fn.call(x)` is
+    // is argument 0 of the `fn.call` call, but also the receiver of a reflective call to `fn`.
+    // It is thus not enough to check if `call` has a known target; we nede to ensure that none of the calls
+    // involving `node1` have a known target.
+    not isArgumentToResolvedCall(node1)
   )
 }

javascript/ql/test/library-tests/FlowSummary/DataFlowConsistency.expected

@@ -199,4 +199,3 @@ multipleArgumentCall
 | tst.js:266:3:266:6 | map3 | tst.js:266:3:266:36 | map3.fo ... value)) | Multiple calls for argument node. |
 lambdaCallEnclosingCallableMismatch
 speculativeStepAlreadyHasModel
-| tst.js:223:39:223:44 | array4 | tst.js:223:12:223:45 | Array.p ... array4) | dispatch |