From 5ed2e033f19ce8886d57b08f9edd109a4fc1f355 Mon Sep 17 00:00:00 2001
From: Asger F <asgerf@github.com>
Date: Mon, 11 Mar 2024 10:43:15 +0100
Subject: [PATCH] JS: More aggressive tracking of objects with methods

---
 .../dataflow/internal/CallGraphs.qll          | 32 +++++++++++++------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
index 4420faacbfe..0e4db6a9a6e 100644
--- a/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/internal/CallGraphs.qll
@@ -263,20 +263,34 @@ module CallGraph {
   }
 
   /**
-   * Gets a step summary for tracking object literals.
-   *
-   * To avoid false flow from callbacks passed in via "named parameters", we only track object
-   * literals out of returns, not into calls.
+   * Holds if `node` has methods but appears in a position where it might be an object
+   * with callbacks pased in via "named parmaeters". This avoid false flow, we do not allow
+   * such objects to be tracked into calls.
    */
-  private StepSummary objectWithMethodsStep() { result = LevelStep() or result = ReturnStep() }
+  pragma[nomagic]
+  private predicate isNamedParameterArgument(DataFlow::ObjectLiteralNode node) {
+    shouldTrackObjectWithMethods(node) and
+    node = any(DataFlow::InvokeNode invoke).getAnArgument()
+  }
+
+  /** Gets a node that refers to the given object, via a limited form of type tracking. */
+  private DataFlow::SourceNode getAnAllocationSiteRef(
+    DataFlow::SourceNode node, DataFlow::TypeTracker t
+  ) {
+    shouldTrackObjectWithMethods(node) and
+    result = node and
+    t.start()
+    or
+    exists(DataFlow::TypeTracker t2 |
+      result = getAnAllocationSiteRef(node, t2).track(t2, t) and
+      (if isNamedParameterArgument(node) then t.hasCall() = false else any())
+    )
+  }
 
   /** Gets a node that refers to the given object, via a limited form of type tracking. */
   cached
   DataFlow::SourceNode getAnAllocationSiteRef(DataFlow::SourceNode node) {
-    shouldTrackObjectWithMethods(node) and
-    result = node
-    or
-    StepSummary::step(getAnAllocationSiteRef(node), result, objectWithMethodsStep())
+    result = getAnAllocationSiteRef(node, DataFlow::TypeTracker::end())
   }
 
   /**