Merge pull request #9535 from github/js-array-filter-taint-step

Tests for rebased 7010

Author: esbena

javascript/ql/lib/semmle/javascript/Arrays.qll

@@ -36,12 +36,18 @@ module ArrayTaintTracking {
       succ = call
     )
     or
-    // `array.filter(x => x)` keeps the taint
+    // `array.filter(x => x)` and `array.filter(x => !!x)` keeps the taint
     call.(DataFlow::MethodCallNode).getMethodName() = "filter" and
     pred = call.getReceiver() and
     succ = call and
-    exists(DataFlow::FunctionNode callback | callback = call.getArgument(0).getAFunctionValue() |
-      callback.getParameter(0).getALocalUse() = callback.getAReturn()
+    exists(DataFlow::FunctionNode callback, DataFlow::Node param, DataFlow::Node ret |
+      callback = call.getArgument(0).getAFunctionValue() and
+      param = callback.getParameter(0).getALocalUse() and
+      ret = callback.getAReturn()
+    |
+      param = ret
+      or
+      param = DataFlow::exprNode(ret.asExpr().(LogNotExpr).getOperand().(LogNotExpr).getOperand())
     )
     or
     // `array.reduce` with tainted value in callback

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

@@ -0,0 +1,26 @@
+| arrays.js:2:16:2:23 | "source" | arrays.js:5:8:5:14 | obj.foo |
+| arrays.js:2:16:2:23 | "source" | arrays.js:11:10:11:15 | arr[i] |
+| 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:49:8:49:13 | arr[0] |
+| 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: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:2:16:2:23 | "source" | arrays.js:84:8:84:17 | arr.at(-1) |
+| 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() |
+| arrays.js:29:21:29:28 | "source" | arrays.js:30:8:30:17 | arr4.pop() |
+| arrays.js:29:21:29:28 | "source" | arrays.js:33:8:33:17 | arr5.pop() |
+| arrays.js:29:21:29:28 | "source" | arrays.js:35:8:35:26 | arr5.slice(2).pop() |
+| arrays.js:29:21:29:28 | "source" | arrays.js:41:8:41:17 | arr6.pop() |
+| arrays.js:44:4:44:11 | "source" | arrays.js:45:10:45:18 | ary.pop() |
+| arrays.js:44:4:44:11 | "source" | arrays.js:46:10:46:12 | ary |
+| arrays.js:86:9:86:16 | "source" | arrays.js:86:8:86:34 | ["sourc ... ) => x) |
+| arrays.js:87:9:87:16 | "source" | arrays.js:87:8:87:36 | ["sourc ... => !!x) |

javascript/ql/test/library-tests/Arrays/TaintFlow.ql

@@ -0,0 +1,15 @@
+import javascript
+
+class ArrayTaintFlowConfig extends TaintTracking::Configuration {
+  ArrayTaintFlowConfig() { this = "ArrayTaintFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source.asExpr().getStringValue() = "source" }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink = any(DataFlow::CallNode call | call.getCalleeName() = "sink").getAnArgument()
+  }
+}
+
+from ArrayTaintFlowConfig config, DataFlow::Node src, DataFlow::Node snk
+where config.hasFlow(src, snk)
+select src, snk

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

@@ -82,4 +82,7 @@
   }
 
   sink(arr.at(-1)); // NOT OK
+
+  sink(["source"].filter((x) => x)); // NOT OK
+  sink(["source"].filter((x) => !!x)); // NOT OK
 });