add more dataflow steps for Arrays

Author: erik-krogh

javascript/ql/src/semmle/javascript/Arrays.qll

@@ -96,10 +96,55 @@ module ArrayTaintTracking {
  * Classes and predicates for modelling data-flow for arrays.
  */
 private module ArrayDataFlow {
+  private import DataFlow::PseudoProperties
+
   /**
-   * Gets a pseudo-field representing an element inside an array.
+   * A step modelling the creation of an Array using the `Array.from(x)` method.
+   * The step copies the elements of the argument (set, array, or iterator elements) into the resulting array.
    */
-  private string arrayElement() { result = "$arrayElement$" }
+  private class ArrayFrom extends DataFlow::AdditionalFlowStep, DataFlow::CallNode {
+    ArrayFrom() { this = DataFlow::globalVarRef("Array").getAMemberCall("from") }
+
+    override predicate loadStoreStep(
+      DataFlow::Node pred, DataFlow::Node succ, string fromProp, string toProp
+    ) {
+      pred = this.getArgument(0) and
+      succ = this and
+      fromProp = arrayLikeElement() and
+      toProp = arrayElement()
+    }
+  }
+
+  /**
+   * A step modelling an array copy where the spread operator is used.
+   * The result is essentially array concatenation.
+   *
+   * Such a step can occur both with the `push` and `unshift` methods, or when creating a new array.
+   */
+  private class ArrayCopySpread extends DataFlow::AdditionalFlowStep {
+    DataFlow::Node spreadArgument; // the spread argument containing the elements to be copied.
+    DataFlow::Node base; // the object where the elements should be copied to.
+
+    ArrayCopySpread() {
+      exists(DataFlow::MethodCallNode mcn | mcn = this |
+        mcn.getMethodName() = ["push", "unshift"] and
+        spreadArgument = mcn.getASpreadArgument() and
+        base = mcn.getReceiver().getALocalSource()
+      )
+      or
+      spreadArgument = this.(DataFlow::ArrayCreationNode).getASpreadArgument() and
+      base = this
+    }
+
+    override predicate loadStoreStep(
+      DataFlow::Node pred, DataFlow::Node succ, string fromProp, string toProp
+    ) {
+      pred = spreadArgument and
+      succ = base and
+      fromProp = arrayLikeElement() and
+      toProp = arrayElement()
+    }
+  }
 
   /**
    * A step for storing an element on an array using `arr.push(e)` or `arr.unshift(e)`.
@@ -112,7 +157,7 @@ private module ArrayDataFlow {
 
     override predicate storeStep(DataFlow::Node element, DataFlow::Node obj, string prop) {
       prop = arrayElement() and
-      (element = this.getAnArgument() or element = this.getASpreadArgument()) and
+      element = this.getAnArgument() and
       obj = this.getReceiver().getALocalSource()
     }
   }
@@ -260,4 +305,20 @@ private module ArrayDataFlow {
       succ = this
     }
   }
+
+  /**
+   * A step for modelling `for of` iteration on arrays.
+   */
+  private class ForOfStep extends DataFlow::AdditionalFlowStep, DataFlow::ValueNode {
+    ForOfStmt forOf;
+    DataFlow::Node element;
+
+    ForOfStep() { this.asExpr() = forOf.getIterationDomain() }
+
+    override predicate loadStep(DataFlow::Node obj, DataFlow::Node e, string prop) {
+      obj = this and
+      e = DataFlow::lvalueNode(forOf.getLValue()) and
+      prop = arrayElement()
+    }
+  }
 }

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

@@ -574,6 +574,22 @@ abstract class AdditionalFlowStep extends DataFlow::Node {
   }
 }
 
+/**
+ * A collection of pseudo-properties that are used in multiple files.
+ * For use with load/store steps in `DataFlow::AdditionalFlowStep` and TypeTracking.
+ */
+module PseudoProperties {
+  /**
+   * Gets a pseudo-field representing an elements inside an `Array`.
+   */
+  string arrayElement() { result = "$arrayElement$" }
+
+  /**
+   * Gets a pseudo-property representing elements inside some array-like object. 
+   */
+  string arrayLikeElement() { result = arrayElement() }
+}
+
 /**
  * A data flow node that should be considered a source for some specific configuration,
  * in addition to any other sources that configuration may recognize.

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

@@ -620,6 +620,16 @@ class ArrayCreationNode extends DataFlow::ValueNode, DataFlow::SourceNode {
     result = this.(ArrayLiteralNode).getSize() or
     result = this.(ArrayConstructorInvokeNode).getSize()
   }
+
+  /** 
+   * Gets a data flow node corresponding to an array of values being passed as
+   * individual arguments to this array creation.
+   */ 
+  DataFlow::Node getASpreadArgument() {
+    exists(SpreadElement arg | arg = getAnElement().getEnclosingExpr() |
+      result = DataFlow::valueNode(arg.getOperand())
+    )
+  }
 }
 
 /**

javascript/ql/test/library-tests/Arrays/DataFlow.expected

@@ -3,6 +3,10 @@
 | arrays.js:2:16:2:23 | "source" | arrays.js:15:27:15:27 | e |
 | arrays.js:2:16:2:23 | "source" | arrays.js:16:23:16:23 | e |
 | arrays.js:2:16:2:23 | "source" | arrays.js:20:8:20:16 | arr.pop() |
+| arrays.js:2:16:2:23 | "source" | arrays.js:52:10:52:10 | x |
+| arrays.js:2:16:2:23 | "source" | arrays.js:56:10:56:10 | x |
+| arrays.js:2:16:2:23 | "source" | arrays.js:60:10:60:10 | x |
+| arrays.js:2:16:2:23 | "source" | arrays.js:66:10:66:10 | x |
 | arrays.js:18:22:18:29 | "source" | arrays.js:18:50:18:50 | e |
 | arrays.js:22:15:22:22 | "source" | arrays.js:23:8:23:17 | arr2.pop() |
 | arrays.js:25:15:25:22 | "source" | arrays.js:26:8:26:17 | arr3.pop() |

javascript/ql/test/library-tests/Arrays/arrays.js

@@ -47,4 +47,22 @@
   });
 
   sink(arr[0]); // OK - tuple like usage. 
+
+  for (const x of arr) {
+    sink(x); // NOT OK
+  }
+
+  for (const x of Array.from(arr)) {
+    sink(x); // NOT OK
+  }
+
+  for (const x of [...arr]) {
+    sink(x); // NOT OK
+  }
+  
+  var arr7 = [];
+  arr7.push(...arr);
+  for (const x of arr7) {
+    sink(x); // NOT OK
+  }
 });