From cffe573d0610a143cac9ebc62355fee0f0e4b16b Mon Sep 17 00:00:00 2001
From: Erik Krogh Kristensen <erik-krogh@github.com>
Date: Tue, 8 Sep 2020 21:30:41 +0200
Subject: [PATCH] add taint-steps for underscore methods

---
 .../frameworks/LodashUnderscore.qll           | 46 +++++++++++++++++++
 .../TaintTracking.expected                    |  6 +++
 .../InterProceduralFlow/underscore.js         | 21 +++++++++
 3 files changed, 73 insertions(+)
 create mode 100644 javascript/ql/test/library-tests/InterProceduralFlow/underscore.js

diff --git a/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll b/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll
index 5461231365f..3e2b3b777b0 100644
--- a/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll
+++ b/javascript/ql/src/semmle/javascript/frameworks/LodashUnderscore.qll
@@ -394,6 +394,52 @@ module LodashUnderscore {
       succ = this.getExceptionalReturn()
     }
   }
+
+  /**
+   * Holds if there is a taint-step involving a (non-function) underscore method from `pred` to `succ`.
+   */
+  private predicate underscoreTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    exists(string name, DataFlow::CallNode call |
+      call = any(Member member | member.getName() = name).getACall()
+    |
+      name =
+        ["find", "filter", "findWhere", "where", "reject", "pluck", "max", "min", "sortBy",
+            "shuffle", "sample", "toArray", "partition", "compact", "first", "initial", "last",
+            "rest", "flatten", "without", "difference", "uniq", "unique", "unzip", "transpose",
+            "object", "chunk", "values", "mapObject", "pick", "omit", "defaults", "clone", "tap",
+            "identity"] and
+      pred = call.getArgument(0) and
+      succ = call
+      or
+      name = ["union", "zip"] and
+      pred = call.getAnArgument() and
+      succ = call
+      or
+      name =
+        ["each", "map", "every", "some", "max", "min", "sortBy", "partition", "mapObject", "tap"] and
+      pred = call.getArgument(0) and
+      succ = call.getABoundCallbackParameter(1, 0)
+      or
+      name = ["reduce", "reduceRight"] and
+      pred = call.getArgument(0) and
+      succ = call.getABoundCallbackParameter(1, 1)
+      or
+      name = ["map", "reduce", "reduceRight"] and
+      pred = call.getCallback(1).getAReturn() and
+      succ = call
+    )
+  }
+
+  /**
+   * A model for taint-steps involving (non-function) underscore methods.
+   */
+  private class UnderscoreTaintStep extends TaintTracking::AdditionalTaintStep {
+    UnderscoreTaintStep() { underscoreTaintStep(this, _) }
+
+    override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+      underscoreTaintStep(pred, succ) and pred = this
+    }
+  }
 }
 
 /**
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
index eeac69e3af1..c3ea1cdb5f2 100644
--- a/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.expected
@@ -86,3 +86,9 @@
 | tst.js:2:17:2:22 | "src1" | tst.js:61:16:61:18 | o.r |
 | tst.js:2:17:2:22 | "src1" | tst.js:68:16:68:22 | inner() |
 | tst.js:2:17:2:22 | "src1" | tst.js:80:16:80:22 | outer() |
+| underscore.js:2:17:2:22 | "src1" | underscore.js:3:15:3:28 | _.max(source1) |
+| underscore.js:5:17:5:22 | "src2" | underscore.js:6:15:6:34 | _.union([], source2) |
+| underscore.js:5:17:5:22 | "src2" | underscore.js:7:15:7:32 | _.zip(source2, []) |
+| underscore.js:9:17:9:22 | "src3" | underscore.js:11:17:11:17 | x |
+| underscore.js:14:17:14:22 | "src4" | underscore.js:16:17:16:17 | e |
+| underscore.js:19:17:19:22 | "src5" | underscore.js:20:15:20:44 | _.map([ ... ource5) |
diff --git a/javascript/ql/test/library-tests/InterProceduralFlow/underscore.js b/javascript/ql/test/library-tests/InterProceduralFlow/underscore.js
new file mode 100644
index 00000000000..858099d28a9
--- /dev/null
+++ b/javascript/ql/test/library-tests/InterProceduralFlow/underscore.js
@@ -0,0 +1,21 @@
+(function() {
+  var source1 = "src1";
+  var sink1 = _.max(source1); // NOT OK
+
+  var source2 = "src2";
+  var sink2 = _.union([], source2); // NOT OK
+  var sink3 = _.zip(source2, []); // NOT OK
+
+  var source3 = "src3";
+  _.map(source3, (x) => {
+    let sink4 = x; // NOT OK
+  });
+
+  var source4 = "src4";
+  _.reduce(source4, (acc, e) => {
+    let sink5 = e; // NOT OK
+  });
+
+  var source5 = "src5";
+  var sink6 = _.map([1,2,3], (x) => source5); // NOT OK
+})();