Merge pull request #2106 from asger-semmle/call-graph-3

JS: Call graph changes

Author: max-schaefer

javascript/ql/src/LanguageFeatures/IllegalInvocation.ql

@@ -61,5 +61,7 @@ where
   forex(DataFlow::InvokeNode cs2, Function otherCallee |
     cs2.getInvokeExpr() = cs.getInvokeExpr() and otherCallee = cs2.getACallee() |
     illegalInvocation(cs, otherCallee, _, _)
-  )
+  ) and
+  // require that all callees are known
+  not cs.isIncomplete()
 select cs, "Illegal invocation of $@ " + how + ".", callee, calleeDesc

javascript/ql/src/meta/analysis-quality/CallGraphQuality.qll

@@ -47,97 +47,6 @@ class RelevantFunction extends Function {
   }
 }
 
-/**
- * Holds if `name` is a the name of an external module.
- */
-predicate isExternalLibrary(string name) {
-  // Mentioned in package.json
-  any(Dependency dep).info(name, _) or
-  // Node.js built-in
-  name = "assert" or
-  name = "async_hooks" or
-  name = "child_process" or
-  name = "cluster" or
-  name = "crypto" or
-  name = "dns" or
-  name = "domain" or
-  name = "events" or
-  name = "fs" or
-  name = "http" or
-  name = "http2" or
-  name = "https" or
-  name = "inspector" or
-  name = "net" or
-  name = "os" or
-  name = "path" or
-  name = "perf_hooks" or
-  name = "process" or
-  name = "punycode" or
-  name = "querystring" or
-  name = "readline" or
-  name = "repl" or
-  name = "stream" or
-  name = "string_decoder" or
-  name = "timer" or
-  name = "tls" or
-  name = "trace_events" or
-  name = "tty" or
-  name = "dgram" or
-  name = "url" or
-  name = "util" or
-  name = "v8" or
-  name = "vm" or
-  name = "worker_threads" or
-  name = "zlib"
-}
-
-/**
- * Holds if the global variable `name` is defined externally.
- */
-predicate isExternalGlobal(string name) {
-  exists(ExternalGlobalDecl decl | decl.getName() = name)
-  or
-  exists(Dependency dep |
-    // If name is never assigned anywhere, and it coincides with a dependency,
-    // it's most likely coming from there.
-    dep.info(name, _) and
-    not exists(Assignment assign | assign.getLhs().(GlobalVarAccess).getName() = name)
-  )
-  or
-  name = "_"
-}
-
-/**
- * Gets a node that was derived from an import of `moduleName`.
- *
- * This is a rough approximation as it follows all property reads, invocations,
- * and callbacks, so some of these might refer to internal objects.
- *
- * Additionally, we don't recognize when a project imports another file in the
- * same project using its module name (for example import "vscode" from inside the vscode project).
- */
-SourceNode externalNode() {
-  exists(string moduleName |
-    result = moduleImport(moduleName) and
-    isExternalLibrary(moduleName)
-  )
-  or
-  exists(string name |
-    result = globalVarRef(name) and
-    isExternalGlobal(name)
-  )
-  or
-  result = DOM::domValueRef()
-  or
-  result = jquery()
-  or
-  result = externalNode().getAPropertyRead()
-  or
-  result = externalNode().getAnInvocation()
-  or
-  result = externalNode().(InvokeNode).getCallback(_).getParameter(_)
-}
-
 /**
  * Gets a data flow node that can be resolved to a function, usually a callback.
  *
@@ -167,7 +76,7 @@ SourceNode nodeLeadingToInvocation() {
     result.flowsTo(arg)
   )
   or
-  exists(AdditionalPartialInvokeNode invoke, Node arg |
+  exists(PartialInvokeNode invoke, Node arg |
     invoke.isPartialArgument(arg, _, _) and
     result.flowsTo(arg)
   )
@@ -192,49 +101,15 @@ class ResolvableCall extends RelevantInvoke {
   }
 }
 
-/**
- * A call site that is believed to call an external function.
- */
-class ExternalCall extends RelevantInvoke {
-  ExternalCall() {
-    not this instanceof ResolvableCall and // avoid double counting
-    (
-      // Call to modelled external library
-      this = externalNode()
-      or
-      // 'require' call or similar
-      this = moduleImport(_)
-      or
-      // Resolved to externs file
-      exists(this.(InvokeNode).getACallee(1))
-      or
-      // Modelled as taint step but isn't from an NPM module, for example, `substring` or `push`.
-      exists(TaintTracking::AdditionalTaintStep step |
-        step.step(_, this)
-        or
-        step.step(this.getAnArgument(), _)
-      )
-    )
-  }
-}
-
 /**
  * A call site that could not be resolved.
  */
 class UnresolvableCall extends RelevantInvoke {
   UnresolvableCall() {
-    not this instanceof ResolvableCall and
-    not this instanceof ExternalCall
+    not this instanceof ResolvableCall
   }
 }
 
-/**
- * A call that is believed to call a function within the same project.
- */
-class NonExternalCall extends RelevantInvoke {
-  NonExternalCall() { not this instanceof ExternalCall }
-}
-
 /**
  * A function with at least one call site.
  */

javascript/ql/src/semmle/javascript/Closure.qll

@@ -248,4 +248,25 @@ module Closure {
   DataFlow::SourceNode moduleImport(string moduleName) {
     getClosureNamespaceFromSourceNode(result) = moduleName
   }
+
+  /**
+   * A call to `goog.bind`, as a partial function invocation.
+   */
+  private class BindCall extends DataFlow::PartialInvokeNode::Range, DataFlow::CallNode {
+    BindCall() { this = moduleImport("goog.bind").getACall() }
+
+    override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
+      index >= 0 and
+      callback = getArgument(0) and
+      argument = getArgument(index + 2)
+    }
+
+    override DataFlow::SourceNode getBoundFunction(DataFlow::Node callback, int boundArgs) {
+      boundArgs = getNumArgument() - 2 and
+      callback = getArgument(0) and
+      result = this
+    }
+
+    override DataFlow::Node getBoundReceiver() { result = getArgument(1) }
+  }
 }

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

@@ -432,18 +432,6 @@ abstract class AdditionalSink extends DataFlow::Node {
   predicate isSinkFor(Configuration cfg, FlowLabel lbl) { none() }
 }
 
-/**
- * An invocation that is modeled as a partial function application.
- *
- * This contributes additional argument-passing flow edges that should be added to all data flow configurations.
- */
-abstract class AdditionalPartialInvokeNode extends DataFlow::InvokeNode {
-  /**
-   * Holds if `argument` is passed as argument `index` to the function in `callback`.
-   */
-  abstract predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index);
-}
-
 /**
  * Additional flow step to model flow from import specifiers into the SSA variable
  * corresponding to the imported variable.
@@ -457,45 +445,6 @@ private class FlowStepThroughImport extends AdditionalFlowStep, DataFlow::ValueN
   }
 }
 
-/**
- * A partial call through the built-in `Function.prototype.bind`.
- */
-private class BindPartialCall extends AdditionalPartialInvokeNode, DataFlow::MethodCallNode {
-  BindPartialCall() { getMethodName() = "bind" }
-
-  override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
-    callback = getReceiver() and
-    argument = getArgument(index + 1)
-  }
-}
-
-/**
- * A partial call through `_.partial`.
- */
-private class LodashPartialCall extends AdditionalPartialInvokeNode {
-  LodashPartialCall() { this = LodashUnderscore::member("partial").getACall() }
-
-  override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
-    callback = getArgument(0) and
-    argument = getArgument(index + 1)
-  }
-}
-
-/**
- * A partial call through `ramda.partial`.
- */
-private class RamdaPartialCall extends AdditionalPartialInvokeNode {
-  RamdaPartialCall() { this = DataFlow::moduleMember("ramda", "partial").getACall() }
-
-  override predicate isPartialArgument(DataFlow::Node callback, DataFlow::Node argument, int index) {
-    callback = getArgument(0) and
-    exists(DataFlow::ArrayCreationNode array |
-      array.flowsTo(getArgument(1)) and
-      argument = array.getElement(index)
-    )
-  }
-}
-
 /**
  * Holds if there is a flow step from `pred` to `succ` described by `summary`
  * under configuration `cfg`.

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -19,6 +19,7 @@
  */
 
 import javascript
+private import internal.CallGraphs
 
 module DataFlow {
   cached
@@ -117,14 +118,23 @@ module DataFlow {
     int getIntValue() { result = asExpr().getIntValue() }
 
     /** Gets a function value that may reach this node. */
-    FunctionNode getAFunctionValue() {
-      result.getAstNode() = analyze().getAValue().(AbstractCallable).getFunction()
+    final FunctionNode getAFunctionValue() {
+      CallGraph::getAFunctionReference(result, 0).flowsTo(this)
+    }
+
+    /** Gets a function value that may reach this node with the given `imprecision` level. */
+    final FunctionNode getAFunctionValue(int imprecision) {
+      CallGraph::getAFunctionReference(result, imprecision).flowsTo(this)
+    }
+
+    /**
+     * Gets a function value that may reach this node,
+     * possibly derived from a partial function invocation.
+     */
+    final FunctionNode getABoundFunctionValue(int boundArgs) {
+      result = getAFunctionValue() and boundArgs = 0
       or
-      exists(string name |
-        GlobalAccessPath::isAssignedInUniqueFile(name) and
-        GlobalAccessPath::fromRhs(result) = name and
-        GlobalAccessPath::fromReference(this) = name
-      )
+      CallGraph::getABoundFunctionReference(result, boundArgs).flowsTo(this)
     }
 
     /**