JS: Support barrier guards that are reflective calls

Author: asger-semmle

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

@@ -308,7 +308,7 @@ abstract class BarrierGuardNode extends DataFlow::Node {
     exists(SsaRefinementNode ref, boolean outcome |
       nd = DataFlow::ssaDefinitionNode(ref) and
       forex(SsaVariable input | input = ref.getAnInput() |
-        asExpr() = ref.getGuard().getTest() and
+        getExpr() = ref.getGuard().getTest() and
         outcome = ref.getGuard().(ConditionGuardNode).getOutcome() and
         barrierGuardBlocksExpr(this, outcome, input.getAUse(), label)
       )
@@ -317,13 +317,20 @@ abstract class BarrierGuardNode extends DataFlow::Node {
     // 2) `nd` is an instance of an access path `p`, and dominated by a barrier for `p`
     exists(AccessPath p, BasicBlock bb, ConditionGuardNode cond, boolean outcome |
       nd = DataFlow::valueNode(p.getAnInstanceIn(bb)) and
-      asExpr() = cond.getTest() and
+      getExpr() = cond.getTest() and
       outcome = cond.getOutcome() and
       barrierGuardBlocksAccessPath(this, outcome, p, label) and
       cond.dominates(bb)
     )
   }
 
+  /** Gets the corresponding expression, including that of reflective calls. */
+  private Expr getExpr() {
+    result = asExpr()
+    or
+    this = DataFlow::reflectiveCallNode(result)
+  }
+
   /**
    * Holds if this node blocks expression `e` provided it evaluates to `outcome`.
    *

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

@@ -932,6 +932,15 @@ module DataFlow {
    */
   DataFlow::Node globalAccessPathRootPseudoNode() { result instanceof TGlobalAccessPathRoot }
 
+  /**
+   * Gets a data flow node representing the underlying call performed by the given
+   * call to `Function.prototype.call` or `Function.prototype.apply`.
+   *
+   * For example, for an expression `fn.call(x, y)`, this gets a call node with `fn` as the
+   * callee, `x` as the receiver, and `y` as the first argument.
+   */
+  DataFlow::InvokeNode reflectiveCallNode(InvokeExpr expr) { result = TReflectiveCallNode(expr, _) }
+
   /**
    * Provides classes representing various kinds of calls.
    *

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -75,6 +75,8 @@ typeInferenceMismatch
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:15:10:15:15 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:21:14:21:19 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:26:9:26:14 | this.x |
+| sanitizer-guards.js:43:11:43:18 | source() | sanitizer-guards.js:45:8:45:8 | x |
+| sanitizer-guards.js:43:11:43:18 | source() | sanitizer-guards.js:48:10:48:10 | x |
 | spread.js:2:15:2:22 | source() | spread.js:4:8:4:19 | { ...taint } |
 | spread.js:2:15:2:22 | source() | spread.js:5:8:5:43 | { f: 'h ... orld' } |
 | spread.js:2:15:2:22 | source() | spread.js:7:8:7:19 | [ ...taint ] |

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.ql

@@ -1,7 +1,11 @@
 import javascript
 import semmle.javascript.dataflow.InferredTypes
 
-DataFlow::CallNode getACall(string name) { result.getCalleeName() = name }
+DataFlow::CallNode getACall(string name) {
+  result.getCalleeName() = name
+  or
+  result.getCalleeNode().getALocalSource() = DataFlow::globalVarRef(name)
+}
 
 class Sink extends DataFlow::Node {
   Sink() { this = getACall("sink").getAnArgument() }

javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

@@ -50,6 +50,9 @@
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:15:10:15:15 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:21:14:21:19 | this.x |
 | sanitizer-guards.js:13:14:13:21 | source() | sanitizer-guards.js:26:9:26:14 | this.x |
+| sanitizer-guards.js:43:11:43:18 | source() | sanitizer-guards.js:45:8:45:8 | x |
+| sanitizer-guards.js:43:11:43:18 | source() | sanitizer-guards.js:48:10:48:10 | x |
+| sanitizer-guards.js:43:11:43:18 | source() | sanitizer-guards.js:52:10:52:10 | x |
 | thisAssignments.js:4:17:4:24 | source() | thisAssignments.js:5:10:5:18 | obj.field |
 | thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 |
 | tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |

javascript/ql/test/library-tests/TaintTracking/sanitizer-guards.js

@@ -38,3 +38,17 @@ class C {
 	});
   }
 }
+
+function reflective() {
+  let x = source();
+
+  sink(x); // NOT OK
+
+  if (isSafe.call(x)) {
+    sink(x); // NOT OK - `isSafe` does not sanitize the receiver
+  }
+
+  if (isSafe.call(null, x)) {
+    sink(x); // OK
+  }
+}