JS: support push and sort taint steps for arrays

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

@@ -214,6 +214,9 @@ module TaintTracking {
           m.getMethodName() = "map" and
           m.getArgument(0) = f and // Require the argument to be a closure to avoid spurious call/return flow
           pred = f.getAReturnedExpr().flow())
+        or
+        // `array.push(e)`: if `e` is tainted, then so is `array`
+        succ.(DataFlow::SourceNode).getAMethodCall("push").getAnArgument() = pred
       )
       or
       // reading from a tainted object yields a tainted result
@@ -508,6 +511,19 @@ module TaintTracking {
     }
   }
 
+  /**
+   * A taint propagating data flow edge arising from sorting.
+   */
+  private class SortTaintStep extends AdditionalTaintStep, DataFlow::MethodCallNode {
+    SortTaintStep() {
+      getMethodName() = "sort"
+    }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      pred = getReceiver() and succ = this
+    }
+  }
+
   /**
    * A conditional checking a tainted string against a regular expression, which is
    * considered to be a sanitizer for all configurations.

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

@@ -1,2 +1,5 @@
 | tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x |
 | tst.js:2:13:2:20 | source() | tst.js:5:10:5:22 | "/" + x + "!" |
+| tst.js:2:13:2:20 | source() | tst.js:14:10:14:17 | x.sort() |
+| tst.js:2:13:2:20 | source() | tst.js:17:10:17:10 | a |
+| tst.js:2:13:2:20 | source() | tst.js:19:10:19:10 | a |

javascript/ql/test/library-tests/TaintTracking/tst.js

@@ -10,4 +10,12 @@ function test() {
     sink(x === 1); // OK
     sink(undefined == x); // OK
     sink(x === x); // OK
+
+    sink(x.sort()); // NOT OK
+
+    var a = [];
+    sink(a); // NOT OK (flow-insensitive treatment of `a`)
+    a.push(x);
+    sink(a); // NOT OK
+
 }