Merge pull request #135 from esben-semmle/js/pick-get-taint-steps

JS: model property projection calls (RC)

change-notes/1.18/analysis-javascript.md

@@ -31,6 +31,8 @@
   - [deepmerge](https://github.com/KyleAMathews/deepmerge)
   - [defaults-deep](https://github.com/jonschlinkert/defaults-deep)
   - [defaults](https://github.com/tmpvar/defaults)
+  - [dottie](https://github.com/mickhansen/dottie.js)
+  - [dotty](https://github.com/deoxxa/dotty)
   - [ent](https://github.com/substack/node-ent)
   - [entities](https://github.com/fb55/node-entities)
   - [escape-goat](https://github.com/sindresorhus/escape-goat)

javascript/ql/src/javascript.qll

@@ -63,6 +63,7 @@ import semmle.javascript.frameworks.Logging
 import semmle.javascript.frameworks.HttpFrameworks
 import semmle.javascript.frameworks.NoSQL
 import semmle.javascript.frameworks.PkgCloud
+import semmle.javascript.frameworks.PropertyProjection
 import semmle.javascript.frameworks.React
 import semmle.javascript.frameworks.ReactNative
 import semmle.javascript.frameworks.Request

javascript/ql/src/semmle/javascript/frameworks/PropertyProjection.qll

@@ -0,0 +1,155 @@
+/**
+ * Provides classes for modelling property projection functions.
+ *
+ * Subclass `PropertyProjection` to refine the behavior of the analysis on existing property projections.
+ * Subclass `CustomPropertyProjection` to introduce new kinds of property projections.
+ */
+
+import javascript
+
+/**
+ * A property projection call such as `_.get(o, 'a.b')`, which is equivalent to `o.a.b`.
+ */
+abstract class CustomPropertyProjection extends DataFlow::CallNode {
+
+  /**
+   * Gets the argument for the object to project properties from, such as `o` in `_.get(o, 'a.b')`.
+   */
+  abstract DataFlow::Node getObject();
+
+  /**
+   * Gets an argument that selects the properties to project, such as `'a.b'` in `_.get(o, 'a.b')`.
+   */
+  abstract DataFlow::Node getASelector();
+
+  /**
+   * Holds if this call returns the value of a single projected property, as opposed to an object that can contain multiple projected properties.
+   */
+  abstract predicate isSingletonProjection();
+
+}
+
+/**
+ * A property projection call such as `_.get(o, 'a.b')`, which is equivalent to `o.a.b`.
+ */
+class PropertyProjection extends DataFlow::CallNode {
+
+  CustomPropertyProjection custom;
+
+  PropertyProjection() { this = custom }
+
+  /**
+   * Gets the argument for the object to project properties from, such as `o` in `_.get(o, 'a.b')`.
+   */
+  DataFlow::Node getObject() { result = custom.getObject() }
+
+  /**
+   * Gets an argument that selects the properties to project, such as `'a.b'` in `_.get(o, 'a.b')`.
+   */
+  DataFlow::Node getASelector() { result = custom.getASelector() }
+
+  /**
+   * Holds if this call returns the value of a single projected property, as opposed to an object that can contain multiple projected properties.
+   *
+   * Examples:
+   * - This predicate holds for `_.get({a: 'b'}, 'a')`, which returns `'b'`,
+   * - This predicate does not hold for `_.pick({a: 'b', c: 'd'}}, 'a')`, which returns `{a: 'b'}`,
+   */
+  predicate isSingletonProjection() { custom.isSingletonProjection() }
+
+}
+
+/**
+ * A simple model of common property projection functions.
+ */
+private class SimplePropertyProjection extends CustomPropertyProjection {
+
+  int objectIndex;
+  int selectorIndex;
+  boolean singleton;
+
+  SimplePropertyProjection() {
+    exists (DataFlow::SourceNode callee |
+      this = callee.getACall() |
+      singleton = false and (
+        (
+          callee = LodashUnderscore::member("pick") and
+          objectIndex = 0 and
+          selectorIndex = [1..getNumArgument()]
+        )
+        or
+        (
+          callee = LodashUnderscore::member("pickBy") and
+          objectIndex = 0 and
+          selectorIndex = 1
+        )
+        or
+        exists (string name |
+          name = "pick" or
+          name = "pickAll" or
+          name = "pickBy" |
+          callee = DataFlow::moduleMember("ramda", name) and
+          objectIndex = 1 and
+          selectorIndex = 0
+        )
+        or
+        (
+          callee = DataFlow::moduleMember("dotty", "search") and
+          objectIndex = 0 and
+          selectorIndex = 1
+        )
+      )
+      or
+      singleton = true and (
+        (
+          callee = LodashUnderscore::member("get") and
+          objectIndex = 0 and
+          selectorIndex = 1
+        )
+        or
+        (
+          callee = DataFlow::moduleMember("ramda", "path") and
+          objectIndex = 1 and
+          selectorIndex = 0
+        )
+        or
+        (
+          callee = DataFlow::moduleMember("dottie", "get") and
+          objectIndex = 0 and
+          selectorIndex = 1
+        )
+        or
+        (
+          callee = DataFlow::moduleMember("dotty", "get") and
+          objectIndex = 0 and
+          selectorIndex = 1
+        )
+      )
+    )
+  }
+
+  override DataFlow::Node getObject() { result = getArgument(objectIndex) }
+
+  override DataFlow::Node getASelector() { result = getArgument(selectorIndex) }
+
+  override predicate isSingletonProjection() { singleton = true }
+
+}
+
+/**
+ * A taint step for a property projection.
+ */
+private class PropertyProjectionTaintStep extends TaintTracking::AdditionalTaintStep {
+
+  PropertyProjection projection;
+
+  PropertyProjectionTaintStep() {
+    projection = this
+  }
+
+  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+    // reading from a tainted object yields a tainted result
+    this = succ and
+    pred = projection.getObject()
+  }
+}

javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.expected

@@ -0,0 +1,3 @@
+| tst.js:25:10:25:15 | source |
+| tst.js:32:10:32:27 | _.pick(tainted, s) |
+| tst.js:33:10:33:26 | _.get(tainted, s) |

javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyInjectionTaint.ql

@@ -0,0 +1,22 @@
+import javascript
+
+class ExampleConfiguration extends TaintTracking::Configuration {
+
+  ExampleConfiguration() { this = "ExampleConfiguration" }
+
+  override predicate isSource(DataFlow::Node source) {
+    source.asExpr().(CallExpr).getCalleeName() = "SOURCE"
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists (CallExpr callExpr |
+      callExpr.getCalleeName() = "SINK" and
+      DataFlow::valueNode(callExpr.getArgument(0)) = sink
+    )
+  }
+
+}
+
+from ExampleConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
+where cfg.hasFlow(source, sink)
+select sink

javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.expected

@@ -0,0 +1,15 @@
+| tst.js:6:1:6:17 | _.pick(o, s1, s2) | tst.js:6:8:6:8 | o | tst.js:6:11:6:12 | s1 | false |
+| tst.js:6:1:6:17 | _.pick(o, s1, s2) | tst.js:6:8:6:8 | o | tst.js:6:15:6:16 | s2 | false |
+| tst.js:7:1:7:14 | _.pickBy(o, s) | tst.js:7:10:7:10 | o | tst.js:7:13:7:13 | s | false |
+| tst.js:9:1:9:12 | R.pick(s, o) | tst.js:9:11:9:11 | o | tst.js:9:8:9:8 | s | false |
+| tst.js:10:1:10:14 | R.pickBy(s, o) | tst.js:10:13:10:13 | o | tst.js:10:10:10:10 | s | false |
+| tst.js:11:1:11:15 | R.pickAll(s, o) | tst.js:11:14:11:14 | o | tst.js:11:11:11:11 | s | false |
+| tst.js:13:1:13:11 | _.get(o, s) | tst.js:13:7:13:7 | o | tst.js:13:10:13:10 | s | true |
+| tst.js:15:1:15:12 | R.path(s, o) | tst.js:15:11:15:11 | o | tst.js:15:8:15:8 | s | true |
+| tst.js:17:1:17:16 | dottie.get(o, s) | tst.js:17:12:17:12 | o | tst.js:17:15:17:15 | s | true |
+| tst.js:19:1:19:15 | dotty.get(o, s) | tst.js:19:11:19:11 | o | tst.js:19:14:19:14 | s | true |
+| tst.js:20:1:20:18 | dotty.search(o, s) | tst.js:20:14:20:14 | o | tst.js:20:17:20:17 | s | false |
+| tst.js:27:10:27:30 | _.pick( ... ted, s) | tst.js:27:17:27:26 | notTainted | tst.js:27:29:27:29 | s | false |
+| tst.js:28:10:28:29 | _.get(notTainted, s) | tst.js:28:16:28:25 | notTainted | tst.js:28:28:28:28 | s | true |
+| tst.js:32:10:32:27 | _.pick(tainted, s) | tst.js:32:17:32:23 | tainted | tst.js:32:26:32:26 | s | false |
+| tst.js:33:10:33:26 | _.get(tainted, s) | tst.js:33:16:33:22 | tainted | tst.js:33:25:33:25 | s | true |

javascript/ql/test/library-tests/frameworks/PropertyProjection/PropertyProjection.ql

@@ -0,0 +1,5 @@
+import javascript
+
+from PropertyProjection p, boolean singleton
+where if p.isSingletonProjection() then singleton = true else singleton = false
+select p, p.getObject(), p.getASelector(), singleton

javascript/ql/test/library-tests/frameworks/PropertyProjection/tst.js

@@ -0,0 +1,34 @@
+var _ = require("lodash"),
+    dotty = require("dotty"),
+    dottie = require("dottie"),
+    R = require("ramda");
+
+_.pick(o, s1, s2);
+_.pickBy(o, s);
+
+R.pick(s, o);
+R.pickBy(s, o);
+R.pickAll(s, o);
+
+_.get(o, s);
+
+R.path(s, o);
+
+dottie.get(o, s);
+
+dotty.get(o, s);
+dotty.search(o, s);
+
+(function(){
+    var source = SOURCE();
+
+    SINK(source);
+
+    SINK(_.pick(notTainted, s));
+    SINK(_.get(notTainted, s));
+
+    var tainted = {};
+    tainted[x] = source;
+    SINK(_.pick(tainted, s));
+    SINK(_.get(tainted, s));
+});