JS: Add convenience layer

Adds getASpreadArgument() and defines getCalleeName() for reflective
calls to be the name of the property being invoked, if there is one.

Author: asger-semmle

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

@@ -841,6 +841,12 @@ module DataFlow {
       /** Gets the data flow node corresponding to an argument of this invocation. */
       abstract DataFlow::Node getAnArgument();
 
+      /**
+       * Gets a data flow node corresponding to an array of values being passed as
+       * individual arguments to this invocation.
+       */
+      abstract DataFlow::Node getASpreadArgument();
+
       /** Gets the number of arguments of this invocation, if it can be determined. */
       abstract int getNumArgument();
     }
@@ -889,6 +895,12 @@ module DataFlow {
         )
       }
 
+      override DataFlow::Node getASpreadArgument() {
+        exists(SpreadElement arg | arg = astNode.getAnArgument() |
+          result = DataFlow::valueNode(arg.getOperand())
+        )
+      }
+
       override int getNumArgument() {
         not astNode.isSpreadArgument(_) and result = astNode.getNumArgument()
       }
@@ -929,7 +941,9 @@ module DataFlow {
 
       ReflectiveCallNodeDef() { this = TReflectiveCallNode(originalCall.asExpr(), kind) }
 
-      override string getCalleeName() { none() }
+      override string getCalleeName() {
+        result = originalCall.getReceiver().asExpr().(PropAccess).getPropertyName()
+      }
 
       override DataFlow::Node getCalleeNode() { result = originalCall.getReceiver() }
 
@@ -943,6 +957,14 @@ module DataFlow {
         kind = "call" and result = originalCall.getAnArgument() and result != getReceiver()
       }
 
+      override DataFlow::Node getASpreadArgument() {
+        kind = "apply" and
+        result = originalCall.getArgument(1)
+        or
+        kind = "call" and
+        result = originalCall.getASpreadArgument()
+      }
+
       override int getNumArgument() {
         result >= 0 and kind = "call" and result = originalCall.getNumArgument() - 1
       }

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

@@ -67,6 +67,19 @@ class InvokeNode extends DataFlow::SourceNode {
   /** Gets the data flow node corresponding to the last argument of this invocation. */
   DataFlow::Node getLastArgument() { result = getArgument(getNumArgument() - 1) }
 
+  /**
+   * Gets a data flow node corresponding to an array of values being passed as
+   * individual arguments to this invocation.
+   *
+   * Example:
+   * ```
+   * x.push(...args);                     // 'args' is a spread argument
+   * x.push(x, ...args, y, ...more);      // 'args' and 'more' are a spread arguments
+   * Array.prototype.push.apply(x, args); // 'args' is a spread argument
+   * ```
+  .*/
+  DataFlow::Node getASpreadArgument() { result = impl.getASpreadArgument() }
+
   /** Gets the number of arguments of this invocation, if it can be determined. */
   int getNumArgument() { result = impl.getNumArgument() }
 

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

@@ -295,8 +295,10 @@ module TaintTracking {
         name = "push" or
         name = "unshift"
       |
-        pred = call.asExpr().(InvokeExpr).getAnArgument().(SpreadElement).getOperand().flow() and
-        succ.(DataFlow::SourceNode).getAMethodCall(name) = call
+        pred = call.getASpreadArgument() and
+        // Make sure we handle reflective calls
+        succ = call.getReceiver().getALocalSource() and
+        call.getCalleeName() = name
       )
       or
       // `array.splice(i, del, e)`: if `e` is tainted, then so is `array`.

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

@@ -12,6 +12,8 @@ typeInferenceMismatch
 | array-mutation.js:19:18:19:25 | source() | array-mutation.js:20:8:20:8 | e |
 | array-mutation.js:23:13:23:20 | source() | array-mutation.js:24:8:24:8 | f |
 | array-mutation.js:27:16:27:23 | source() | array-mutation.js:28:8:28:8 | g |
+| array-mutation.js:31:33:31:40 | source() | array-mutation.js:32:8:32:8 | h |
+| array-mutation.js:35:36:35:43 | source() | array-mutation.js:36:8:36:8 | i |
 | booleanOps.js:2:11:2:18 | source() | booleanOps.js:4:8:4:8 | x |
 | booleanOps.js:2:11:2:18 | source() | booleanOps.js:13:10:13:10 | x |
 | booleanOps.js:2:11:2:18 | source() | booleanOps.js:19:10:19:10 | x |

javascript/ql/test/library-tests/TaintTracking/array-mutation.js

@@ -26,4 +26,12 @@ function test(x, y) {
   let g = [];
   g.unshift(...source());
   sink(g); // NOT OK
+
+  let h = [];
+  Array.prototype.push.apply(h, source());
+  sink(h); // NOT OK
+
+  let i = [];
+  Array.prototype.unshift.apply(i, source());
+  sink(i); // NOT OK
 }