From 22b56a4a40761e6168cc4ac04610d2dd69fffe6f Mon Sep 17 00:00:00 2001
From: Asger F <asgerf@github.com>
Date: Thu, 7 Mar 2024 11:51:06 +0100
Subject: [PATCH] JS: More implied receiver steps

---
 .../dataflow/internal/CallGraphs.qll          | 23 +++++++++++++++++++
 .../CallGraphs/AnnotatedTest/Test.expected    |  1 -
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
index 7e55944038bf..541e3a6f3e90 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
@@ -279,6 +279,20 @@ module CallGraph {
     StepSummary::step(getAnAllocationSiteRef(node), result, objectWithMethodsStep())
   }
 
+  /**
+   * Holds if `function` flows to a property of `host` via non-local data flow.
+   */
+  pragma[nomagic]
+  private predicate complexMethodInstallation(
+    DataFlow::SourceNode host, DataFlow::FunctionNode function
+  ) {
+    not function = getAMethodOnObject(_) and
+    exists(DataFlow::TypeTracker t |
+      getAFunctionReference(function, 0, t) = host.getAPropertySource() and
+      t.start() // require call bit to be false
+    )
+  }
+
   /**
    * Holds if `pred` is assumed to flow to `succ` because a method is stored on an object that is assumed
    * to be the receiver of calls to that method.
@@ -291,9 +305,18 @@ module CallGraph {
    */
   cached
   predicate impliedReceiverStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) {
+    // To avoid double-recursion, we handle either complex flow for the host object, or for the function, but not both.
     exists(DataFlow::SourceNode host |
+      // Complex flow for the host object
       pred = getAnAllocationSiteRef(host) and
       succ = getAMethodOnObject(host).getReceiver()
+      or
+      // Complex flow for the function
+      exists(DataFlow::FunctionNode function |
+        complexMethodInstallation(host, function) and
+        pred = host and
+        succ = function.getReceiver()
+      )
     )
   }
 }
diff --git a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected
index 9d59da5ccadd..8182d0174140 100644
--- a/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected
+++ b/javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected
@@ -2,7 +2,6 @@ spuriousCallee
 missingCallee
 | constructor-field.ts:40:5:40:14 | f3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls |
 | constructor-field.ts:71:1:71:11 | bf3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 | calls |
-| implied-receiver.js:7:13:7:25 | this.member() | implied-receiver.js:17:22:19:1 | functio ... n 42;\\n} | -1 | calls |
 badAnnotation
 accessorCall
 | accessors.js:12:1:12:5 | obj.f | accessors.js:5:8:5:12 | () {} |