JS: Improve flow through partial invokes

Author: asgerf

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

@@ -267,6 +267,9 @@ module Closure {
       result = this
     }
 
-    override DataFlow::Node getBoundReceiver() { result = getArgument(1) }
+    override DataFlow::Node getBoundReceiver(DataFlow::Node callback) {
+      callback = getArgument(0) and
+      result = getArgument(1)
+    }
   }
 }

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

@@ -1311,6 +1311,9 @@ class MidPathNode extends PathNode, MkMidNode {
     or
     // Skip the exceptional return on functions, as this highlights the entire function.
     nd = any(DataFlow::FunctionNode f).getExceptionalReturn()
+    or
+    // Skip the synthetic 'this' node, as a ThisExpr will be the next node anyway
+    nd = DataFlow::thisNode(_)
   }
 }
 

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

@@ -1199,6 +1199,13 @@ class PartialInvokeNode extends DataFlow::Node {
 
   PartialInvokeNode() { this = range }
 
+  /** Gets a node holding a callback invoked by this partial invocation node. */
+  DataFlow::Node getACallbackNode() {
+    isPartialArgument(result, _, _)
+    or
+    exists(getBoundReceiver(result))
+  }
+
   /**
    * Holds if `argument` is passed as argument `index` to the function in `callback`.
    */
@@ -1216,7 +1223,12 @@ class PartialInvokeNode extends DataFlow::Node {
   /**
    * Gets the node holding the receiver to be passed to the bound function, if specified.
    */
-  DataFlow::Node getBoundReceiver() { result = range.getBoundReceiver() }
+  DataFlow::Node getBoundReceiver() { result = range.getBoundReceiver(_) }
+
+  /**
+   * Gets the node holding the receiver to be passed to the bound function, if specified.
+   */
+  DataFlow::Node getBoundReceiver(DataFlow::Node callback) { result = range.getBoundReceiver(callback) }
 }
 
 module PartialInvokeNode {
@@ -1235,9 +1247,17 @@ module PartialInvokeNode {
     DataFlow::SourceNode getBoundFunction(DataFlow::Node callback, int boundArgs) { none() }
 
     /**
+     * DEPRECATED. Use the two-argument version of `getBoundReceiver` instead.
+     *
      * Gets the node holding the receiver to be passed to the bound function, if specified.
      */
+    deprecated
     DataFlow::Node getBoundReceiver() { none() }
+
+    /**
+     * Gets the node holding the receiver to be passed to `callback`.
+     */
+    DataFlow::Node getBoundReceiver(DataFlow::Node callback) { none() }
   }
 
   /**
@@ -1264,7 +1284,8 @@ module PartialInvokeNode {
       result = this
     }
 
-    override DataFlow::Node getBoundReceiver() {
+    override DataFlow::Node getBoundReceiver(DataFlow::Node callback) {
+      callback = getReceiver() and
       result = getArgument(0)
     }
   }
@@ -1309,6 +1330,22 @@ module PartialInvokeNode {
       result = this
     }
   }
+
+  /**
+   * A call to `for-in` or `for-own`, passing the context parameter to the target function.
+   */
+  class ForOwnInPartialCall extends PartialInvokeNode::Range, DataFlow::CallNode {
+    ForOwnInPartialCall() {
+      exists(string name | name = "for-in" or name = "for-own" |
+        this = moduleImport(name).getACall()
+      )
+    }
+
+    override DataFlow::Node getBoundReceiver(DataFlow::Node callback) {
+      callback = getArgument(1) and
+      result = getArgument(2)
+    }
+  }
 }
 
 /**

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

@@ -99,7 +99,7 @@ private module CachedSteps {
   private predicate partiallyCalls(
     DataFlow::PartialInvokeNode invk, DataFlow::AnalyzedNode callback, Function f
   ) {
-    invk.isPartialArgument(callback, _, _) and
+    callback = invk.getACallbackNode() and
     exists(AbstractFunction callee | callee = callback.getAValue() |
       if callback.getAValue().isIndefinite("global")
       then f = callee.getFunction() and f.getFile() = invk.getFile()
@@ -135,6 +135,12 @@ private module CachedSteps {
       not p.isRestParameter() and
       parm = DataFlow::parameterNode(p)
     )
+    or
+    exists(DataFlow::Node callback |
+      arg = invk.(DataFlow::PartialInvokeNode).getBoundReceiver(callback) and
+      partiallyCalls(invk, callback, f) and
+      parm = DataFlow::thisNode(f)
+    )
   }
 
   /**

javascript/ql/src/semmle/javascript/frameworks/AngularJS/AngularJSCore.qll

@@ -1097,5 +1097,8 @@ private class BindCall extends DataFlow::PartialInvokeNode::Range, DataFlow::Cal
     result = this
   }
 
-  override DataFlow::Node getBoundReceiver() { result = getArgument(0) }
+  override DataFlow::Node getBoundReceiver(DataFlow::Node callback) {
+    callback = getArgument(1) and
+    result = getArgument(0)
+  }
 }