JS: Refine criteria so that explicit this-passing is not affected

Author: asgerf

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -1325,8 +1325,8 @@ module API {
         prev = trackUseNode(nd, promisified, boundArgs, prop, t) and
         StepSummary::step(prev, res, summary) and
         result = t.append(summary) and
-        // Block argument-passing into 'this'
-        not (summary = CallStep() and res instanceof DataFlow::ThisNode)
+        // Block argument-passing into 'this' when it determines the call target
+        not summary = CallReceiverStep()
       )
     }
 
@@ -1384,8 +1384,8 @@ module API {
         next = trackDefNode(nd, t) and
         StepSummary::step(prev, next, summary) and
         result = t.prepend(summary) and
-        // Block argument-passing steps from 'this' back to a receiver
-        not (summary = CallStep() and prev instanceof DataFlow::ThisNode)
+        // Block argument-passing steps from 'this' back to a receiver when it determines the call target
+        not summary = CallReceiverStep()
       )
     }
 

javascript/ql/lib/semmle/javascript/dataflow/TypeTracking.qll

@@ -65,6 +65,8 @@ class TypeTracker extends TTypeTracker {
     or
     step = CallStep() and result = MkTypeTracker(true, prop)
     or
+    step = CallReceiverStep() and result = MkTypeTracker(true, prop)
+    or
     step = ReturnStep() and hasCall = false and result = this
     or
     step = LoadStep(prop) and result = MkTypeTracker(hasCall, "")
@@ -238,6 +240,8 @@ class TypeBackTracker extends TTypeBackTracker {
     or
     step = CallStep() and hasReturn = false and result = this
     or
+    step = CallReceiverStep() and hasReturn = false and result = this
+    or
     step = ReturnStep() and result = MkTypeBackTracker(true, prop)
     or
     exists(string p | step = LoadStep(p) and prop = "" and result = MkTypeBackTracker(hasReturn, p))

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

@@ -43,6 +43,7 @@ private module Cached {
     newtype TStepSummary =
       LevelStep() or
       CallStep() or
+      CallReceiverStep() or
       ReturnStep() or
       StoreStep(PropertyName prop) or
       LoadStep(PropertyName prop) or
@@ -101,6 +102,15 @@ private module Cached {
     )
   }
 
+  pragma[nomagic]
+  private predicate isReceiverForMethodDispatch(DataFlow::Node node) {
+    exists(DataFlow::SourceNode base, DataFlow::CallNode invoke |
+      node = invoke.getReceiver() and
+      base = node.getALocalSource() and
+      invoke.getCalleeNode() = base.getAPropertyRead()
+    )
+  }
+
   /**
    * INTERNAL: Use `TypeBackTracker.smallstep()` instead.
    */
@@ -116,7 +126,11 @@ private module Cached {
     or
     // Flow into function
     callStep(pred, succ) and
-    summary = CallStep()
+    (
+      if isReceiverForMethodDispatch(pred)
+      then summary = CallReceiverStep()
+      else summary = CallStep()
+    )
     or
     // Flow out of function
     returnStep(pred, succ) and
@@ -251,6 +265,8 @@ class StepSummary extends TStepSummary {
     or
     this instanceof CallStep and result = "call"
     or
+    this instanceof CallReceiverStep and result = "call-receiver"
+    or
     this instanceof ReturnStep and result = "return"
     or
     exists(string prop | this = StoreStep(prop) | result = "store " + prop)

javascript/ql/test/ApiGraphs/explicit-this/VerifyAssertions.expected

@@ -1 +0,0 @@
-| tst.js:4:17:4:119 | /** use ... rn() */ | use moduleImport("something").getMember("exports").getMember("one") has no outgoing edge labelled getMember("two"); it has no outgoing edges at all. |