Merge pull request #20586 from asgerf/js/api-graphs-block-this

JS: Restrict receiver-flow in API graphs

Author: asgerf

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

@@ -1324,7 +1324,9 @@ module API {
       exists(DataFlow::TypeTracker t, StepSummary summary, DataFlow::SourceNode prev |
         prev = trackUseNode(nd, promisified, boundArgs, prop, t) and
         StepSummary::step(prev, res, summary) and
-        result = t.append(summary)
+        result = t.append(summary) and
+        // Block argument-passing into 'this' when it determines the call target
+        not summary = CallReceiverStep()
       )
     }
 
@@ -1381,7 +1383,9 @@ module API {
       exists(DataFlow::TypeBackTracker t, StepSummary summary, DataFlow::Node next |
         next = trackDefNode(nd, t) and
         StepSummary::step(prev, next, summary) and
-        result = t.prepend(summary)
+        result = t.prepend(summary) and
+        // 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

@@ -0,0 +1 @@
+import ApiGraphs.VerifyAssertions

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

@@ -0,0 +1,6 @@
+{
+    "name": "explicit-this",
+    "dependencies": {
+        "something": "*"
+    }
+}

javascript/ql/test/ApiGraphs/explicit-this/package.json

@@ -0,0 +1,7 @@
+const lib = require('something');
+
+function f() {
+    this.two(); /** use=moduleImport("something").getMember("exports").getMember("one").getMember("two").getReturn() */
+}
+
+f.call(lib.one);