Merge pull request #15823 from asgerf/js/lift-cg-restriction

JS: Call graph improvements

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

@@ -1262,6 +1262,12 @@ module ClassNode {
     result.getFile() = f
   }
 
+  pragma[nomagic]
+  private DataFlow::NewNode getAnInstantiationInFile(string name, File f) {
+    result = AccessPath::getAReferenceTo(name).(DataFlow::LocalSourceNode).getAnInstantiation() and
+    result.getFile() = f
+  }
+
   /**
    * Gets a reference to the function `func`, where there exists a read/write of the "prototype" property on that reference.
    */
@@ -1273,7 +1279,7 @@ module ClassNode {
   }
 
   /**
-   * A function definition with prototype manipulation as a `ClassNode` instance.
+   * A function definition, targeted by a `new`-call or with prototype manipulation, seen as a `ClassNode` instance.
    */
   class FunctionStyleClass extends Range, DataFlow::ValueNode {
     override Function astNode;
@@ -1284,9 +1290,12 @@ module ClassNode {
       (
         exists(getAFunctionValueWithPrototype(function))
         or
-        exists(string name |
-          this = AccessPath::getAnAssignmentTo(name) and
+        function = any(NewNode new).getCalleeNode().analyze().getAValue()
+        or
+        exists(string name | this = AccessPath::getAnAssignmentTo(name) |
           exists(getAPrototypeReferenceInFile(name, this.getFile()))
+          or
+          exists(getAnInstantiationInFile(name, this.getFile()))
         )
       )
     }

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

@@ -241,24 +241,25 @@ module CallGraph {
     )
   }
 
-  private DataFlow::FunctionNode getAMethodOnPlainObject(DataFlow::SourceNode node) {
+  private DataFlow::FunctionNode getAMethodOnObject(DataFlow::SourceNode node) {
     (
-      (
-        node instanceof DataFlow::ObjectLiteralNode
-        or
-        node instanceof DataFlow::FunctionNode
-      ) and
       result = node.getAPropertySource()
       or
       result = node.(DataFlow::ObjectLiteralNode).getPropertyGetter(_)
       or
       result = node.(DataFlow::ObjectLiteralNode).getPropertySetter(_)
     ) and
-    not node.getTopLevel().isExterns()
+    not node.getTopLevel().isExterns() and
+    // Ignore writes to `this` inside a constructor, since this is already handled by instance method tracking
+    not exists(DataFlow::ClassNode cls |
+      node = cls.getConstructor().getReceiver()
+      or
+      node = cls.(DataFlow::ClassNode::FunctionStyleClass).getAPrototypeReference()
+    )
   }
 
   private predicate shouldTrackObjectWithMethods(DataFlow::SourceNode node) {
-    exists(getAMethodOnPlainObject(node))
+    exists(getAMethodOnObject(node))
   }
 
   /**
@@ -292,7 +293,7 @@ module CallGraph {
   predicate impliedReceiverStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) {
     exists(DataFlow::SourceNode host |
       pred = getAnAllocationSiteRef(host) and
-      succ = getAMethodOnPlainObject(host).getReceiver()
+      succ = getAMethodOnObject(host).getReceiver()
     )
   }
 }

javascript/ql/src/change-notes/2024-03-07-lift-cg-restriction.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The call graph has been improved, leading to more alerts for data flow based queries.

javascript/ql/test/library-tests/CallGraphs/FullTest/tests.expected

@@ -135,6 +135,7 @@ test_getAFunctionValue
 | tst.js:3:1:3:1 | h | tst.js:3:5:3:17 | function() {} |
 | tst.js:3:1:3:17 | h = function() {} | tst.js:3:5:3:17 | function() {} |
 | tst.js:3:5:3:17 | function() {} | tst.js:3:5:3:17 | function() {} |
+| tst.js:4:1:4:1 | k | tst.js:2:9:2:21 | function() {} |
 | tst.js:4:1:4:5 | k = g | tst.js:2:9:2:21 | function() {} |
 | tst.js:4:5:4:5 | g | tst.js:2:9:2:21 | function() {} |
 | tst.js:6:1:6:1 | f | tst.js:1:1:1:15 | function f() {} |
@@ -142,13 +143,23 @@ test_getAFunctionValue
 | tst.js:8:1:8:1 | h | tst.js:3:5:3:17 | function() {} |
 | tst.js:9:1:9:1 | k | tst.js:2:9:2:21 | function() {} |
 | tst.js:11:1:20:1 | functio ... \\tf();\\n} | tst.js:11:1:20:1 | functio ... \\tf();\\n} |
+| tst.js:11:12:11:12 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:11:12:11:12 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:12:6:12:6 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:12:6:12:27 | n | tst.js:2:9:2:21 | function() {} |
 | tst.js:12:6:12:27 | n | tst.js:12:15:12:27 | function() {} |
+| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:12:10:12:10 | m | tst.js:2:9:2:21 | function() {} |
+| tst.js:12:10:12:27 | m \|\| function() {} | tst.js:2:9:2:21 | function() {} |
 | tst.js:12:10:12:27 | m \|\| function() {} | tst.js:12:15:12:27 | function() {} |
 | tst.js:12:15:12:27 | function() {} | tst.js:12:15:12:27 | function() {} |
 | tst.js:13:2:13:16 | function p() {} | tst.js:13:2:13:16 | function p() {} |
 | tst.js:13:11:13:11 | p | tst.js:13:2:13:16 | function p() {} |
+| tst.js:14:2:14:2 | m | tst.js:2:9:2:21 | function() {} |
 | tst.js:15:2:15:2 | l | tst.js:11:1:20:1 | functio ... \\tf();\\n} |
 | tst.js:16:2:16:17 | arguments.callee | tst.js:11:1:20:1 | functio ... \\tf();\\n} |
+| tst.js:17:2:17:2 | n | tst.js:2:9:2:21 | function() {} |
 | tst.js:17:2:17:2 | n | tst.js:12:15:12:27 | function() {} |
 | tst.js:18:2:18:2 | p | tst.js:13:2:13:16 | function p() {} |
 | tst.js:19:2:19:2 | f | tst.js:1:1:1:15 | function f() {} |
@@ -463,8 +474,10 @@ test_getACallee
 | tst.js:7:1:7:3 | g() | tst.js:2:9:2:21 | function() {} |
 | tst.js:8:1:8:3 | h() | tst.js:3:5:3:17 | function() {} |
 | tst.js:9:1:9:3 | k() | tst.js:2:9:2:21 | function() {} |
+| tst.js:14:2:14:4 | m() | tst.js:2:9:2:21 | function() {} |
 | tst.js:15:2:15:4 | l() | tst.js:11:1:20:1 | functio ... \\tf();\\n} |
 | tst.js:16:2:16:19 | arguments.callee() | tst.js:11:1:20:1 | functio ... \\tf();\\n} |
+| tst.js:17:2:17:4 | n() | tst.js:2:9:2:21 | function() {} |
 | tst.js:17:2:17:4 | n() | tst.js:12:15:12:27 | function() {} |
 | tst.js:18:2:18:4 | p() | tst.js:13:2:13:16 | function p() {} |
 | tst.js:19:2:19:4 | f() | tst.js:1:1:1:15 | function f() {} |