JS: Track flow into calls to bound functions

Author: asgerf

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

@@ -147,7 +147,7 @@ module DataFlow {
     final FunctionNode getABoundFunctionValue(int boundArgs) {
       result = getAFunctionValue() and boundArgs = 0
       or
-      CallGraph::getABoundFunctionReference(result, boundArgs).flowsTo(this)
+      CallGraph::getABoundFunctionReference(result, boundArgs, _).flowsTo(this)
     }
 
     /**

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

@@ -74,7 +74,7 @@ module CallGraph {
    */
   pragma[nomagic]
   private
-  DataFlow::SourceNode getABoundFunctionReference(DataFlow::FunctionNode function, int boundArgs, DataFlow::TypeTracker t) {
+  DataFlow::SourceNode getABoundFunctionReferenceAux(DataFlow::FunctionNode function, int boundArgs, DataFlow::TypeTracker t) {
     exists(DataFlow::PartialInvokeNode partial, DataFlow::Node callback |
       result = partial.getBoundFunction(callback, boundArgs) and
       getAFunctionReference(function, 0, t.continue()).flowsTo(callback)
@@ -90,7 +90,7 @@ module CallGraph {
   private
   DataFlow::SourceNode getABoundFunctionReferenceAux(DataFlow::FunctionNode function, int boundArgs, DataFlow::TypeTracker t, DataFlow::StepSummary summary) {
     exists(DataFlow::SourceNode prev |
-      prev = getABoundFunctionReference(function, boundArgs, t) and
+      prev = getABoundFunctionReferenceAux(function, boundArgs, t) and
       DataFlow::StepSummary::step(prev, result, summary)
     )
   }
@@ -100,8 +100,12 @@ module CallGraph {
    * with `function` as the underlying function.
    */
   cached
-  DataFlow::SourceNode getABoundFunctionReference(DataFlow::FunctionNode function, int boundArgs) {
-    result = getABoundFunctionReference(function, boundArgs, DataFlow::TypeTracker::end())
+  DataFlow::SourceNode getABoundFunctionReference(DataFlow::FunctionNode function, int boundArgs, boolean contextDependent) {
+    exists(DataFlow::TypeTracker t |
+      result = getABoundFunctionReferenceAux(function, boundArgs, t) and
+      t.end() and
+      contextDependent = t.hasCall()
+    )
   }
 
   /**

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

@@ -6,6 +6,7 @@
 
 import javascript
 import semmle.javascript.dataflow.Configuration
+import semmle.javascript.dataflow.internal.CallGraphs
 
 /**
  * Holds if flow should be tracked through properties of `obj`.
@@ -91,6 +92,18 @@ private module CachedSteps {
   cached
   predicate calls(DataFlow::InvokeNode invk, Function f) { f = invk.getACallee(0) }
 
+  /**
+   * Holds if `invk` may invoke a bound version of `f` with `boundArgs` already bound.
+   *
+   * The receiver is assumed to be bound as well, and should not propagate into `f`.
+   *
+   * Does not hold for context-dependent call sites, such as callback invocations.
+   */
+  cached
+  predicate callsBound(DataFlow::InvokeNode invk, Function f, int boundArgs) {
+    CallGraph::getABoundFunctionReference(f.flow(), boundArgs, false).flowsTo(invk.getCalleeNode())
+  }
+
   /**
    * Holds if `invk` may invoke `f` indirectly through the given `callback` argument.
    *
@@ -141,6 +154,14 @@ private module CachedSteps {
       partiallyCalls(invk, callback, f) and
       parm = DataFlow::thisNode(f)
     )
+    or
+    exists(int boundArgs, int i, Parameter p |
+      callsBound(invk, f, boundArgs) and
+      f.getParameter(boundArgs + i) = p and
+      not p.isRestParameter() and
+      arg = invk.getArgument(i) and
+      parm = DataFlow::parameterNode(p)
+    )
   }
 
   /**

javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.expected

@@ -1,5 +1,5 @@
 spuriousCallee
 missingCallee
-| constructor-field.ts:40:5:40:14 | f3.build() | constructor-field.ts:13:3:13:12 | build() {} |
-| constructor-field.ts:71:1:71:11 | bf3.build() | constructor-field.ts:13:3:13:12 | build() {} |
+| constructor-field.ts:40:5:40:14 | f3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 |
+| constructor-field.ts:71:1:71:11 | bf3.build() | constructor-field.ts:13:3:13:12 | build() {} | -1 |
 badAnnotation

javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/Test.ql

@@ -33,16 +33,37 @@ class AnnotatedCall extends InvokeExpr {
   string getCallTargetName() { result = calls }
 
   AnnotatedFunction getAnExpectedCallee() { result.getCalleeName() = getCallTargetName() }
+
+  int getBoundArgs() {
+    result = getAnnotation(this, "boundArgs").toInt()
+  }
+
+  int getBoundArgsOrMinusOne() {
+    result = getBoundArgs()
+    or
+    not exists(getBoundArgs()) and
+    result = -1
+  }
+}
+
+predicate callEdge(AnnotatedCall call, AnnotatedFunction target, int boundArgs) {
+  FlowSteps::calls(call.flow(), target) and boundArgs = -1
+  or
+  FlowSteps::callsBound(call.flow(), target, boundArgs)
 }
 
-query predicate spuriousCallee(AnnotatedCall call, AnnotatedFunction target) {
-  FlowSteps::calls(call.flow(), target) and
-  not target = call.getAnExpectedCallee()
+query predicate spuriousCallee(AnnotatedCall call, AnnotatedFunction target, int boundArgs) {
+  callEdge(call, target, boundArgs) and
+  not (
+    target = call.getAnExpectedCallee() and
+    boundArgs = call.getBoundArgsOrMinusOne()
+  )
 }
 
-query predicate missingCallee(AnnotatedCall call, AnnotatedFunction target) {
-  not FlowSteps::calls(call.flow(), target) and
-  target = call.getAnExpectedCallee()
+query predicate missingCallee(AnnotatedCall call, AnnotatedFunction target, int boundArgs) {
+  not callEdge(call, target, boundArgs) and
+  target = call.getAnExpectedCallee() and
+  boundArgs = call.getBoundArgsOrMinusOne()
 }
 
 query predicate badAnnotation(string name) {

javascript/ql/test/library-tests/CallGraphs/AnnotatedTest/bound-function.js

@@ -0,0 +1,31 @@
+var url = require('url')
+var http = require('http')
+
+function Mount () {}
+
+/** name:mount.serve */
+Mount.prototype.serve = function (x) {
+}
+
+function makeMount() {
+  var m = new Mount()
+  return m.serve.bind(m);
+}
+
+function makeMount2(x) {
+  var m = new Mount()
+  return m.serve.bind(m, x);
+}
+
+var mount = makeMount()
+var mount2 = makeMount2(null);
+
+http.createServer(function (req, res) {
+  /** calls:mount.serve */
+  /** boundArgs:0 */
+  mount(req, res)
+
+  /** calls:mount.serve */
+  /** boundArgs:1 */
+  mount2(res)
+});