Merge pull request #1258 from asger-semmle/prototype-pollution

JS: prototype pollution query template

javascript/ql/src/Security/CWE-400/RemotePropertyInjection.expected

@@ -0,0 +1 @@
+| examples/RemotePropertyInjection.js:8:8:8:11 | prop | A $@ is used as a property name to write to. | examples/RemotePropertyInjection.js:7:13:7:36 | req.que ... trolled | user-provided value |

javascript/ql/src/semmle/javascript/security/dataflow/PrototypePollution.qll

@@ -0,0 +1,138 @@
+/**
+ * Provides a taint-tracking configuration for tracking user-controlled objects flowing
+ * into a vulnerable `extends` call.
+ */
+
+import javascript
+import semmle.javascript.security.TaintedObject
+
+module PrototypePollution {
+  /**
+   * Label for wrappers around tainted objects, that is, objects that are
+   * not completely user-controlled, but contain a user-controlled object.
+   *
+   * For example, `options` below is is a tainted wrapper, but is not itself
+   * a tainted object:
+   * ```
+   * let options = {
+   *   prefs: {
+   *     locale: req.query.locale
+   *   }
+   * }
+   * ```
+   */
+  module TaintedObjectWrapper {
+    private class TaintedObjectWrapper extends DataFlow::FlowLabel {
+      TaintedObjectWrapper() { this = "tainted-object-wrapper" }
+    }
+
+    TaintedObjectWrapper label() { any() }
+  }
+
+  /**
+   * A data flow source for prototype pollution.
+   */
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets the type of data coming from this source.
+     */
+    abstract DataFlow::FlowLabel getAFlowLabel();
+  }
+
+  /**
+   * A data flow sink for prototype pollution.
+   */
+  abstract class Sink extends DataFlow::Node {
+    /**
+     * Gets the type of data that can taint this sink.
+     */
+    abstract DataFlow::FlowLabel getAFlowLabel();
+  }
+
+  /**
+   * A taint tracking configuration for user-controlled objects flowing into deep `extend` calls,
+   * leading to prototype pollution.
+   */
+  class Configuration extends TaintTracking::Configuration {
+    Configuration() { this = "PrototypePollution" }
+
+    override predicate isSource(DataFlow::Node node, DataFlow::FlowLabel label) {
+      node.(Source).getAFlowLabel() = label
+    }
+
+    override predicate isSink(DataFlow::Node node, DataFlow::FlowLabel label) {
+      node.(Sink).getAFlowLabel() = label
+    }
+
+    override predicate isAdditionalFlowStep(
+      DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel inlbl, DataFlow::FlowLabel outlbl
+    ) {
+      TaintedObject::step(src, dst, inlbl, outlbl)
+      or
+      // Track objects are wrapped in other objects
+      exists(DataFlow::PropWrite write |
+        src = write.getRhs() and
+        inlbl = TaintedObject::label() and
+        dst = write.getBase().getALocalSource() and
+        outlbl = TaintedObjectWrapper::label()
+      )
+    }
+
+    override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode node) {
+      node instanceof TaintedObject::SanitizerGuard
+    }
+  }
+
+  /**
+   * A user-controlled string value, as a source of prototype pollution.
+   *
+   * Note that values from this type of source will need to flow through a `JSON.parse` call
+   * in order to be flagged for prototype pollution.
+   */
+  private class RemoteFlowAsSource extends Source {
+    RemoteFlowAsSource() { this instanceof RemoteFlowSource }
+
+    override DataFlow::FlowLabel getAFlowLabel() { result = DataFlow::FlowLabel::data() }
+  }
+
+  /**
+   * A source of user-controlled objects.
+   */
+  private class TaintedObjectSource extends Source {
+    TaintedObjectSource() { this instanceof TaintedObject::Source }
+
+    override DataFlow::FlowLabel getAFlowLabel() { result = TaintedObject::label() }
+  }
+  
+  string getModuleName(ExtendCall call) {
+    call = DataFlow::moduleImport(result).getACall() or
+    call = DataFlow::moduleMember(result, _).getACall()
+  }
+
+  class DeepExtendSink extends Sink {
+    ExtendCall call;
+
+    DeepExtendSink() {
+      this = call.getASourceOperand() and
+      call.isDeep() and
+      exists(string moduleName | moduleName = getModuleName(call) |
+        moduleName = "lodash" + any(string s) or
+        moduleName = "just-extend" or
+        moduleName = "extend" or
+        moduleName = "extend2" or
+        moduleName = "node.extend" or
+        moduleName = "merge" or
+        moduleName = "smart-extend" or
+        moduleName = "js-extend" or
+        moduleName = "deep" or
+        moduleName = "defaults-deep"
+      )
+    }
+
+    override DataFlow::FlowLabel getAFlowLabel() {
+      result = TaintedObject::label()
+      or
+      result = TaintedObjectWrapper::label()
+    }
+  }
+}

javascript/ql/test/query-tests/Security/CWE-400/PrototypePollution.expected

@@ -0,0 +1,15 @@
+nodes
+| PrototypePollution.js:7:17:7:29 | req.query.foo |
+| PrototypePollution.js:10:17:12:5 | {\\n      ... K\\n    } |
+| PrototypePollution.js:11:16:11:30 | req.query.value |
+| PrototypePollution.js:15:14:15:28 | req.query.value |
+| PrototypePollution.js:17:17:19:5 | {\\n      ... K\\n    } |
+| PrototypePollution.js:18:16:18:25 | opts.thing |
+edges
+| PrototypePollution.js:11:16:11:30 | req.query.value | PrototypePollution.js:10:17:12:5 | {\\n      ... K\\n    } |
+| PrototypePollution.js:15:14:15:28 | req.query.value | PrototypePollution.js:18:16:18:25 | opts.thing |
+| PrototypePollution.js:18:16:18:25 | opts.thing | PrototypePollution.js:17:17:19:5 | {\\n      ... K\\n    } |
+#select
+| PrototypePollution.js:7:17:7:29 | req.query.foo | PrototypePollution.js:7:17:7:29 | req.query.foo | PrototypePollution.js:7:17:7:29 | req.query.foo | Prototype pollution caused by merging a user-controlled value from $@. | PrototypePollution.js:7:17:7:29 | req.query.foo | here |
+| PrototypePollution.js:10:17:12:5 | {\\n      ... K\\n    } | PrototypePollution.js:11:16:11:30 | req.query.value | PrototypePollution.js:10:17:12:5 | {\\n      ... K\\n    } | Prototype pollution caused by merging a user-controlled value from $@. | PrototypePollution.js:11:16:11:30 | req.query.value | here |
+| PrototypePollution.js:17:17:19:5 | {\\n      ... K\\n    } | PrototypePollution.js:15:14:15:28 | req.query.value | PrototypePollution.js:17:17:19:5 | {\\n      ... K\\n    } | Prototype pollution caused by merging a user-controlled value from $@. | PrototypePollution.js:15:14:15:28 | req.query.value | here |

javascript/ql/test/query-tests/Security/CWE-400/PrototypePollution.js

@@ -0,0 +1,21 @@
+let express = require('express');
+let _ = require('lodash');
+
+let app = express();
+
+app.get('/hello', function(req, res) {
+    _.merge({}, req.query.foo); // NOT OK
+    _.merge({}, req.query); // NOT OK - but not flagged
+    
+    _.merge({}, {
+        value: req.query.value // NOT OK
+    });
+    
+    let opts = {
+      thing: req.query.value // wrapped and unwrapped value
+    };
+    _.merge({}, {
+        value: opts.thing // NOT OK
+    });
+});
+

javascript/ql/test/query-tests/Security/CWE-400/PrototypePollution.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Prototype Pollution
+ * @description Recursively merging a user-controlled object into another object
+ *              can allow an attacker to modify the built-in Object prototype. 
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/prototype-pollution
+ * @tags security
+ *       external/cwe/cwe-250
+ *       external/cwe/cwe-400
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.PrototypePollution::PrototypePollution
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
+where cfg.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Prototype pollution caused by merging a user-controlled value from $@.", source, "here"