Merge pull request #6294 from erik-krogh/arrify

Approved by asgerf

Author: codeql-ci

javascript/change-notes/2021-07-15-array-libs.md

@@ -0,0 +1,15 @@
+lgtm,codescanning
+* The dataflow libraries now model dataflow through more array libraries.
+  Affected packages are
+    [array-from](https://npmjs.com/package/array-from),
+    [array.prototype.find](https://npmjs.com/package/array.prototype.find),
+    [array-find](https://npmjs.com/package/array-find),
+    [arrify](https://npmjs.com/package/arrify),
+    [array-ify](https://npmjs.com/package/array-ify),
+    [array-union](https://npmjs.com/package/array-union),
+    [array-uniq](https://npmjs.com/package/array-uniq),
+    [uniq](https://npmjs.com/package/uniq),
+    [array-flatten](https://npmjs.com/package/array-flatten),
+    [arr-flatten](https://npmjs.com/package/arr-flatten),
+    [flatten](https://npmjs.com/package/flatten),
+    [array.prototype.flat](https://npmjs.com/package/array.prototype.flat)

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

@@ -68,7 +68,7 @@ module ArrayTaintTracking {
     succ = call
     or
     // `e = Array.from(x)`: if `x` is tainted, then so is `e`.
-    call = DataFlow::globalVarRef("Array").getAPropertyRead("from").getACall() and
+    call = arrayFromCall() and
     pred = call.getAnArgument() and
     succ = call
     or
@@ -79,6 +79,11 @@ module ArrayTaintTracking {
     call.(DataFlow::MethodCallNode).getMethodName() = "concat" and
     succ = call and
     pred = call.getAnArgument()
+    or
+    // find
+    // `e = arr.find(callback)`
+    call = arrayFindCall(pred) and
+    succ = call
   }
 }
 
@@ -97,7 +102,7 @@ private module ArrayDataFlow {
       DataFlow::Node pred, DataFlow::Node succ, string fromProp, string toProp
     ) {
       exists(DataFlow::CallNode call |
-        call = DataFlow::globalVarRef("Array").getAMemberCall("from") and
+        call = arrayFromCall() and
         pred = call.getArgument(0) and
         succ = call and
         fromProp = arrayLikeElement() and
@@ -297,4 +302,108 @@ private module ArrayDataFlow {
       )
     }
   }
+
+  /**
+   * A step modelling that elements from an array `arr` are received by calling `find`.
+   */
+  private class ArrayFindStep extends DataFlow::SharedFlowStep {
+    override predicate loadStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+      exists(DataFlow::CallNode call |
+        call = arrayFindCall(pred) and
+        succ = call and
+        prop = arrayElement()
+      )
+    }
+  }
+}
+
+private import ArrayLibraries
+
+/**
+ * Classes and predicates modelling various libraries that work on arrays or array-like structures.
+ */
+private module ArrayLibraries {
+  private import DataFlow::PseudoProperties
+
+  /**
+   * Gets a call to `Array.from` or a polyfill implementing the same functionality.
+   */
+  DataFlow::CallNode arrayFromCall() {
+    result = DataFlow::globalVarRef("Array").getAMemberCall("from")
+    or
+    result = DataFlow::moduleImport("array-from").getACall()
+  }
+
+  /**
+   * Gets a call to `Array.prototype.find` or a polyfill implementing the same functionality.
+   */
+  DataFlow::CallNode arrayFindCall(DataFlow::Node array) {
+    result.(DataFlow::MethodCallNode).getMethodName() = "find" and
+    array = result.getReceiver()
+    or
+    result = DataFlow::moduleImport(["array.prototype.find", "array-find"]).getACall() and
+    array = result.getArgument(0)
+  }
+
+  /**
+   * A taint step through the `arrify` library, or other libraries that (maybe) convert values into arrays.
+   */
+  private class ArrayifyStep extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(API::CallNode call | call = API::moduleImport(["arrify", "array-ify"]).getACall() |
+        pred = call.getArgument(0) and succ = call
+      )
+    }
+  }
+
+  /**
+   * A call to a library that copies the elements of an array into another array.
+   * E.g. `array-union` that creates a union of multiple arrays, or `array-uniq` that creates an array with unique elements.
+   */
+  DataFlow::CallNode arrayCopyCall(DataFlow::Node array) {
+    result = API::moduleImport(["array-union", "array-uniq", "uniq"]).getACall() and
+    array = result.getAnArgument()
+  }
+
+  /**
+   * A taint step for a library that copies the elements of an array into another array.
+   */
+  private class ArrayCopyTaint extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call |
+        call = arrayCopyCall(pred) and
+        succ = call
+      )
+    }
+  }
+
+  /**
+   * A loadStoreStep for a library that copies the elements of an array into another array.
+   */
+  private class ArrayCopyLoadStore extends DataFlow::SharedFlowStep {
+    override predicate loadStoreStep(DataFlow::Node pred, DataFlow::Node succ, string prop) {
+      exists(DataFlow::CallNode call |
+        call = arrayCopyCall(pred) and
+        succ = call and
+        prop = arrayElement()
+      )
+    }
+  }
+
+  /**
+   * A taint step through a call to `Array.prototype.flat` or a polyfill implementing array flattening.
+   */
+  private class ArrayFlatStep extends TaintTracking::SharedTaintStep {
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      exists(DataFlow::CallNode call | succ = call |
+        call.(DataFlow::MethodCallNode).getMethodName() = "flat" and
+        pred = call.getReceiver()
+        or
+        call =
+          API::moduleImport(["array-flatten", "arr-flatten", "flatten", "array.prototype.flat"])
+              .getACall() and
+        pred = call.getAnArgument()
+      )
+    }
+  }
 }

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

@@ -7,6 +7,10 @@
 | 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:2:16:2:23 | "source" | arrays.js:71:10:71:10 | x |
+| arrays.js:2:16:2:23 | "source" | arrays.js:74:8:74:29 | arr.fin ... llback) |
+| arrays.js:2:16:2:23 | "source" | arrays.js:77:8:77:35 | arrayFi ... llback) |
+| arrays.js:2:16:2:23 | "source" | arrays.js:81:10:81: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

@@ -65,4 +65,19 @@
   for (const x of arr7) {
     sink(x); // NOT OK
   }
+
+  const arrayFrom = require("array-from");
+  for (const x of arrayFrom(arr)) {
+    sink(x); // NOT OK
+  }
+
+  sink(arr.find(someCallback)); // NOT OK
+
+  const arrayFind = require("array-find");
+  sink(arrayFind(arr, someCallback)); // NOT OK
+
+  const uniq = require("uniq");
+  for (const x of uniq(arr)) {
+    sink(x); // NOT OK
+  }
 });