Merge pull request #2617 from asger-semmle/prototype-pollution-utility

Approved by esbena, mchammer01

change-notes/1.24/analysis-javascript.md

@@ -21,6 +21,7 @@
 | Cross-site scripting through exception (`js/xss-through-exception`) | security, external/cwe/cwe-079, external/cwe/cwe-116              | Highlights potential XSS vulnerabilities where an exception is written to the DOM. Results are not shown on LGTM by default. |
 | Regular expression always matches (`js/regex/always-matches`) | correctness, regular-expressions | Highlights regular expression checks that trivially succeed by matching an empty substring. Results are shown on LGTM by default. |
 | Missing await (`js/missing-await`) | correctness | Highlights expressions that operate directly on a promise object in a nonsensical way, instead of awaiting its result. Results are shown on LGTM by default. |
+| Prototype pollution in utility function (`js/prototype-pollution-utility`) | security, external/cwe/cwe-400, external/cwe/cwe-471 | Highlights recursive copying operations that are susceptible to prototype pollution. Results are shown on LGTM by default. |
 
 ## Changes to existing queries
 

javascript/config/suites/javascript/security

@@ -31,6 +31,7 @@
 + semmlecode-javascript-queries/Security/CWE-352/MissingCsrfMiddleware.ql: /Security/CWE/CWE-352
 + semmlecode-javascript-queries/Security/CWE-400/RemotePropertyInjection.ql: /Security/CWE/CWE-400
 + semmlecode-javascript-queries/Security/CWE-400/PrototypePollution.ql: /Security/CWE/CWE-400
++ semmlecode-javascript-queries/Security/CWE-400/PrototypePollutionUtility.ql: /Security/CWE/CWE-400
 + semmlecode-javascript-queries/Security/CWE-502/UnsafeDeserialization.ql: /Security/CWE/CWE-502
 + semmlecode-javascript-queries/Security/CWE-506/HardcodedDataInterpretedAsCode.ql: /Security/CWE/CWE-506
 + semmlecode-javascript-queries/Security/CWE-601/ClientSideUrlRedirect.ql: /Security/CWE/CWE-601

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.qhelp

@@ -0,0 +1,71 @@
+<!DOCTYPE qhelp PUBLIC
+"-//Semmle//qhelp//EN"
+"qhelp.dtd">
+<qhelp>
+
+<overview>
+  <p>
+    Most JavaScript objects inherit the properties of the built-in <code>Object.prototype</code> object.
+    Prototype pollution is a type of vulnerability in which an attacker is able to modify <code>Object.prototype</code>.
+    Since most objects inherit from the compromised <code>Object.prototype</code>, the attacker can use this
+    to tamper with the application logic, and often escalate to remote code execution or cross-site scripting.
+  </p>
+
+  <p>
+    One way to cause prototype pollution is through use of an unsafe <em>merge</em> or <em>extend</em> function
+    to recursively copy properties from one object to another.
+    Such a function has the potential to modify any object reachable from the destination object, and
+    the built-in <code>Object.prototype</code> is usually reachable through the special properties
+    <code>__proto__</code> and <code>constructor.prototype</code>.
+  </p>
+</overview>
+
+<recommendation>
+  <p>
+    The most effective place to guard against this is in the function that performs
+    the recursive copy.
+  </p>
+
+  <p>
+    Only merge a property recursively when it is an own property of the <em>destination</em> object.
+    Alternatively, blacklist the property names <code>__proto__</code> and <code>constructor</code>
+    from being merged.
+  </p>
+</recommendation>
+
+<example>
+  <p>
+    This function recursively copies properties from <code>src</code> to <code>dst</code>:
+  </p>
+
+  <sample src="examples/PrototypePollutionUtility.js"/>
+
+  <p>
+    However, if <code>src</code> is the object <code>{"__proto__": {"isAdmin": true}}</code>,
+    it will inject the property <code>isAdmin: true</code> in <code>Object.prototype</code>.
+  </p>
+
+  <p>
+    The issue can be fixed by ensuring that only own properties of the destination object
+    are merged recursively:
+  </p>
+
+  <sample src="examples/PrototypePollutionUtility_fixed.js"/>
+
+  <p>
+    Alternatively, blacklist the <code>__proto__</code> and <code>constructor</code> properties:
+  </p>
+
+  <sample src="examples/PrototypePollutionUtility_fixed2.js"/>
+</example>
+
+<references>
+  <li>Prototype pollution attacks:
+    <a href="https://hackerone.com/reports/380873">lodash</a>,
+    <a href="https://hackerone.com/reports/454365">jQuery</a>,
+    <a href="https://hackerone.com/reports/381185">extend</a>,
+    <a href="https://hackerone.com/reports/430291">just-extend</a>,
+    <a href="https://hackerone.com/reports/381194">merge.recursive</a>.
+  </li>
+</references>
+</qhelp>

javascript/ql/src/Security/CWE-400/PrototypePollutionUtility.ql

@@ -0,0 +1,566 @@
+/**
+ * @name Prototype pollution in utility function
+ * @description Recursively copying properties between objects may cause
+                accidental modification of a built-in prototype object.
+ * @kind path-problem
+ * @problem.severity warning
+ * @precision high
+ * @id js/prototype-pollution-utility
+ * @tags security
+ *       external/cwe/cwe-400
+ *       external/cwe/cwe-471
+ */
+
+import javascript
+import DataFlow
+import PathGraph
+import semmle.javascript.dataflow.InferredTypes
+
+/**
+ * Gets a node that refers to an element of `array`, likely obtained
+ * as a result of enumerating the elements of the array.
+ */
+SourceNode getAnEnumeratedArrayElement(SourceNode array) {
+  exists(MethodCallNode call, string name |
+    call = array.getAMethodCall(name) and
+    (name = "forEach" or name = "map") and
+    result = call.getCallback(0).getParameter(0)
+  )
+  or
+  exists(DataFlow::PropRead read |
+    read = array.getAPropertyRead() and
+    not exists(read.getPropertyName()) and
+    not read.getPropertyNameExpr().analyze().getAType() = TTString() and
+    result = read
+  )
+}
+
+/**
+ * A data flow node that refers to the name of a property obtained by enumerating
+ * the properties of some object.
+ */
+abstract class EnumeratedPropName extends DataFlow::Node {
+  /**
+   * Gets the object whose properties are being enumerated.
+   *
+   * For example, gets `src` in `for (var key in src)`.
+   */
+  abstract DataFlow::Node getSourceObject();
+
+  /**
+   * Gets a local reference of the source object.
+   */
+  SourceNode getASourceObjectRef() {
+    exists(SourceNode root, string path |
+      getSourceObject() = AccessPath::getAReferenceTo(root, path) and
+      result = AccessPath::getAReferenceTo(root, path)
+    )
+    or
+    result = getSourceObject().getALocalSource()
+  }
+
+  /**
+   * Gets a property read that accesses the corresponding property value in the source object.
+   *
+   * For example, gets `src[key]` in `for (var key in src) { src[key]; }`.
+   */
+  PropRead getASourceProp() {
+    result = getASourceObjectRef().getAPropertyRead() and
+    result.getPropertyNameExpr().flow().getImmediatePredecessor*() = this
+  }
+}
+
+/**
+ * Property enumeration through `for-in` for `Object.keys` or similar.
+ */
+class ForInEnumeratedPropName extends EnumeratedPropName {
+  DataFlow::Node object;
+
+  ForInEnumeratedPropName() {
+    exists(ForInStmt stmt |
+      this = DataFlow::lvalueNode(stmt.getLValue()) and
+      object = stmt.getIterationDomain().flow()
+    )
+    or
+    exists(CallNode call |
+      call = globalVarRef("Object").getAMemberCall("keys")
+      or
+      call = globalVarRef("Object").getAMemberCall("getOwnPropertyNames")
+      or
+      call = globalVarRef("Reflect").getAMemberCall("ownKeys")
+    |
+      object = call.getArgument(0) and
+      this = getAnEnumeratedArrayElement(call)
+    )
+  }
+
+  override Node getSourceObject() { result = object }
+}
+
+/**
+ * Property enumeration through `Object.entries`.
+ */
+class EntriesEnumeratedPropName extends EnumeratedPropName {
+  CallNode entries;
+  SourceNode entry;
+
+  EntriesEnumeratedPropName() {
+    entries = globalVarRef("Object").getAMemberCall("entries") and
+    entry = getAnEnumeratedArrayElement(entries) and
+    this = entry.getAPropertyRead("0")
+  }
+
+  override DataFlow::Node getSourceObject() {
+    result = entries.getArgument(0)
+  }
+
+  override PropRead getASourceProp() {
+    result = super.getASourceProp()
+    or
+    result = entry.getAPropertyRead("1")
+  }
+}
+
+/**
+ * Holds if the properties of `node` are enumerated locally.
+ */
+predicate arePropertiesEnumerated(DataFlow::SourceNode node) {
+  node = any(EnumeratedPropName name).getASourceObjectRef()
+}
+
+/**
+ * A dynamic property access that is not obviously an array access.
+ */
+class DynamicPropRead extends DataFlow::SourceNode, DataFlow::ValueNode {
+  // Use IndexExpr instead of PropRead as we're not interested in implicit accesses like
+  // rest-patterns and for-of loops.
+  override IndexExpr astNode;
+
+  DynamicPropRead() {
+    not exists(astNode.getPropertyName()) and
+    // Exclude obvious array access
+    astNode.getPropertyNameExpr().analyze().getAType() = TTString()
+  }
+
+  /** Gets the base of the dynamic read. */
+  DataFlow::Node getBase() { result = astNode.getBase().flow() }
+
+  /**
+   * Holds if the value of this read was assigned to earlier in the same basic block.
+   *
+   * For example, this is true for `dst[x]` on line 2 below:
+   * ```js
+   * dst[x] = {};
+   * dst[x][y] = src[y];
+   * ```
+   */
+  predicate hasDominatingAssignment() {
+    exists(DataFlow::PropWrite write, BasicBlock bb, int i, int j, SsaVariable ssaVar |
+      write = getBase().getALocalSource().getAPropertyWrite() and
+      bb.getNode(i) = write.getWriteNode() and
+      bb.getNode(j) = astNode and
+      i < j and
+      write.getPropertyNameExpr() = ssaVar.getAUse() and
+      astNode.getIndex() = ssaVar.getAUse()
+    )
+  }
+}
+
+/**
+ * Holds if there is a dynamic property assignment of form `base[prop] = rhs`
+ * which might act as the writing operation in a recursive merge function.
+ *
+ * Only assignments to pre-existing objects are of interest, so object/array literals
+ * are not included.
+ *
+ * Additionally, we ignore cases where the properties of `base` are enumerated, as this
+ * would typically not happen in a merge function.
+ */
+predicate dynamicPropWrite(DataFlow::Node base, DataFlow::Node prop, DataFlow::Node rhs) {
+  exists(AssignExpr write, IndexExpr index |
+    index = write.getLhs() and
+    base = index.getBase().flow() and
+    prop = index.getPropertyNameExpr().flow() and
+    rhs = write.getRhs().flow() and
+    not exists(prop.getStringValue()) and
+    not arePropertiesEnumerated(base.getALocalSource())
+  )
+}
+
+/** Gets the name of a property that can lead to `Object.prototype`. */
+string unsafePropName() {
+  result = "__proto__"
+  or
+  result = "constructor"
+}
+
+/**
+ * Flow label representing an unsafe property name, or an object obtained
+ * by using such a property in a dynamic read.
+ */
+class UnsafePropLabel extends FlowLabel {
+  UnsafePropLabel() { this = unsafePropName() }
+}
+
+/**
+ * Tracks data from property enumerations to dynamic property writes.
+ *
+ * The intent is to find code of the general form:
+ * ```js
+ * function merge(dst, src) {
+ *   for (var key in src)
+ *     if (...)
+ *       merge(dst[key], src[key])
+ *     else
+ *       dst[key] = src[key]
+ * }
+ * ```
+ *
+ * This configuration is used to find three separate data flow paths originating
+ * from a property enumeration, all leading to the same dynamic property write.
+ *
+ * In particular, the base and property name of the property write should all
+ * depend on the enumerated property name (`key`) and the right-hand side should
+ * depend on the source property (`src[key]`), while allowing steps of form
+ * `x -> x[p]` and `p -> x[p]`.
+ *
+ * Note that in the above example, the flow from `key` to the base of the write (`dst`)
+ * requires stepping through the recursive call.
+ * Such a path would be absent for a shallow copying operation, where the `dst` object
+ * isn't derived from a property of the source object.
+ *
+ * This configuration can't enforce that all three paths must end at the same
+ * dynamic property write, so we treat the paths independently here and check
+ * for coinciding paths afterwards.  This means this configuration can't be used as
+ * a standalone configuration like in most path queries.
+ */
+class PropNameTracking extends DataFlow::Configuration {
+  PropNameTracking() { this = "PropNameTracking" }
+
+  override predicate isSource(DataFlow::Node node, FlowLabel label) {
+    label instanceof UnsafePropLabel and
+    exists(EnumeratedPropName prop |
+      node = prop
+      or
+      node = prop.getASourceProp()
+    )
+  }
+
+  override predicate isSink(DataFlow::Node node, FlowLabel label) {
+    label instanceof UnsafePropLabel and
+    (
+      dynamicPropWrite(node, _, _) or
+      dynamicPropWrite(_, node, _) or
+      dynamicPropWrite(_, _, node)
+    )
+  }
+
+  override predicate isAdditionalFlowStep(
+    DataFlow::Node pred, DataFlow::Node succ, FlowLabel predlbl, FlowLabel succlbl
+  ) {
+    predlbl instanceof UnsafePropLabel and
+    succlbl = predlbl and
+    (
+      // Step through `p -> x[p]`
+      exists(PropRead read |
+        pred = read.getPropertyNameExpr().flow() and
+        not read.(DynamicPropRead).hasDominatingAssignment() and
+        succ = read
+      )
+      or
+      // Step through `x -> x[p]`
+      exists(DynamicPropRead read |
+        not read.hasDominatingAssignment() and
+        pred = read.getBase() and
+        succ = read
+      )
+    )
+  }
+
+  override predicate isBarrier(DataFlow::Node node) {
+    super.isBarrier(node)
+    or
+    exists(ConditionGuardNode guard, SsaRefinementNode refinement |
+      node = DataFlow::ssaDefinitionNode(refinement) and
+      refinement.getGuard() = guard and
+      guard.getTest() instanceof VarAccess and
+      guard.getOutcome() = false
+    )
+  }
+
+  override predicate isBarrierGuard(DataFlow::BarrierGuardNode node) {
+    node instanceof BlacklistEqualityGuard or
+    node instanceof WhitelistEqualityGuard or
+    node instanceof HasOwnPropertyGuard or
+    node instanceof InExprGuard or
+    node instanceof InstanceOfGuard or
+    node instanceof TypeofGuard or
+    node instanceof BlacklistInclusionGuard or
+    node instanceof WhitelistInclusionGuard
+  }
+}
+
+/**
+ * Sanitizer guard of form `x === "__proto__"` or `x === "constructor"`.
+ */
+class BlacklistEqualityGuard extends DataFlow::LabeledBarrierGuardNode, ValueNode {
+  override EqualityTest astNode;
+  string propName;
+
+  BlacklistEqualityGuard() {
+    astNode.getAnOperand().getStringValue() = propName and
+    propName = unsafePropName()
+  }
+
+  override predicate blocks(boolean outcome, Expr e, FlowLabel label) {
+    e = astNode.getAnOperand() and
+    outcome = astNode.getPolarity().booleanNot() and
+    label = propName
+  }
+}
+
+/**
+ * An equality test with something other than `__proto__` or `constructor`.
+ */
+class WhitelistEqualityGuard extends DataFlow::LabeledBarrierGuardNode, ValueNode {
+  override EqualityTest astNode;
+
+  WhitelistEqualityGuard() {
+    not astNode.getAnOperand().getStringValue() = unsafePropName() and
+    astNode.getAnOperand() instanceof Literal
+  }
+
+  override predicate blocks(boolean outcome, Expr e, FlowLabel label) {
+    e = astNode.getAnOperand() and
+    outcome = astNode.getPolarity() and
+    label instanceof UnsafePropLabel
+  }
+}
+
+/**
+ * Sanitizer guard for calls to `Object.prototype.hasOwnProperty`.
+ *
+ * A malicious source object will have `__proto__` and/or `constructor` as own properties,
+ * but the destination object generally doesn't. It is therefore only a sanitizer when
+ * used on the destination object.
+ */
+class HasOwnPropertyGuard extends DataFlow::BarrierGuardNode, CallNode {
+  HasOwnPropertyGuard() {
+    // Make sure we handle reflective calls since libraries love to do that.
+    getCalleeNode().getALocalSource().(DataFlow::PropRead).getPropertyName() = "hasOwnProperty" and
+    exists(getReceiver()) and
+    // Try to avoid `src.hasOwnProperty` by requiring that the receiver
+    // does not locally have its properties enumerated. Typically there is no
+    // reason to enumerate the properties of the destination object.
+    not arePropertiesEnumerated(getReceiver().getALocalSource())
+  }
+
+  override predicate blocks(boolean outcome, Expr e) {
+    e = getArgument(0).asExpr() and outcome = true
+  }
+}
+
+/**
+ * Sanitizer guard for `key in dst`.
+ *
+ * Since `"__proto__" in obj` and `"constructor" in obj` is true for most objects,
+ * this is seen as a sanitizer for `key` in the false outcome.
+ */
+class InExprGuard extends DataFlow::BarrierGuardNode, DataFlow::ValueNode {
+  override InExpr astNode;
+
+  InExprGuard() {
+    // Exclude tests of form `key in src` for the same reason as in HasOwnPropertyGuard
+    not arePropertiesEnumerated(astNode.getRightOperand().flow().getALocalSource())
+  }
+
+  override predicate blocks(boolean outcome, Expr e) {
+    e = astNode.getLeftOperand() and outcome = false
+  }
+}
+
+/**
+ * Sanitizer guard for `instanceof` expressions.
+ *
+ * `Object.prototype instanceof X` is never true, so this blocks the `__proto__` label.
+ *
+ * It is still possible to get to `Function.prototype` through `constructor.constructor.prototype`
+ * so we do not block the `constructor` label.
+ */
+class InstanceOfGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::ValueNode {
+  override InstanceOfExpr astNode;
+
+  override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+    e = astNode.getLeftOperand() and outcome = true and label = "__proto__"
+  }
+}
+
+/**
+ * Sanitizer guard of form `typeof x === "object"` or `typeof x === "function"`.
+ *
+ * The former blocks the `constructor` label as that payload must pass through a function,
+ * and the latter blocks the `__proto__` label as that only passes through objects.
+ */
+class TypeofGuard extends DataFlow::LabeledBarrierGuardNode, DataFlow::ValueNode {
+  override EqualityTest astNode;
+  TypeofExpr typeof;
+  string typeofStr;
+
+  TypeofGuard() {
+    typeof = astNode.getAnOperand() and
+    typeofStr = astNode.getAnOperand().getStringValue()
+  }
+
+  override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+    e = typeof.getOperand() and
+    outcome = astNode.getPolarity() and
+    (
+      typeofStr = "object" and
+      label = "constructor"
+      or
+      typeofStr = "function" and
+      label = "__proto__"
+    )
+    or
+    e = typeof.getOperand() and
+    outcome = astNode.getPolarity().booleanNot() and
+    (
+      // If something is not an object, sanitize object, as both must end
+      // in non-function prototype object.
+      typeofStr = "object" and
+      label instanceof UnsafePropLabel
+      or
+      typeofStr = "function" and
+      label = "constructor"
+    )
+  }
+}
+
+/**
+ * A check of form `["__proto__"].includes(x)` or similar.
+ */
+class BlacklistInclusionGuard extends DataFlow::LabeledBarrierGuardNode, InclusionTest {
+  UnsafePropLabel label;
+
+  BlacklistInclusionGuard() {
+    exists(DataFlow::ArrayCreationNode array |
+      array.getAnElement().getStringValue() = label and
+      array.flowsTo(getContainerNode())
+    )
+  }
+
+  override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel lbl) {
+    outcome = getPolarity().booleanNot() and
+    e = getContainedNode().asExpr() and
+    label = lbl
+  }
+}
+
+/**
+ * A check of form `xs.includes(x)` or similar, which sanitizes `x` in the true case.
+ */
+class WhitelistInclusionGuard extends DataFlow::LabeledBarrierGuardNode {
+  WhitelistInclusionGuard() {
+    this instanceof TaintTracking::PositiveIndexOfSanitizer or
+    this instanceof TaintTracking::InclusionSanitizer
+  }
+
+  override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel lbl) {
+    this.(TaintTracking::AdditionalSanitizerGuardNode).sanitizes(outcome, e) and
+    lbl instanceof UnsafePropLabel
+  }
+}
+
+/**
+ * Gets a meaningful name for `node` if possible.
+ */
+string getExprName(DataFlow::Node node) {
+  result = node.asExpr().(Identifier).getName()
+  or
+  result = node.asExpr().(DotExpr).getPropertyName()
+}
+
+/**
+ * Gets a name to display for `node`.
+ */
+string deriveExprName(DataFlow::Node node) {
+  result = getExprName(node)
+  or
+  not exists(getExprName(node)) and
+  result = "this object"
+}
+
+/**
+ * Holds if the dynamic property write `base[prop] = rhs` can pollute the prototype
+ * of `base` due to flow from `enum`.
+ *
+ * In most cases this will result in an alert, the exception being the case where
+ * `base` does not have a prototype at all.
+ */
+predicate isPrototypePollutingAssignment(Node base, Node prop, Node rhs, EnumeratedPropName enum) {
+  dynamicPropWrite(base, prop, rhs) and
+  exists(PropNameTracking cfg |
+    cfg.hasFlow(enum, base) and
+    cfg.hasFlow(enum, prop) and
+    cfg.hasFlow(enum.getASourceProp(), rhs)
+  )
+}
+
+/** Gets a data flow node leading to the base of a prototype-polluting assignment. */
+private DataFlow::SourceNode getANodeLeadingToBase(DataFlow::TypeBackTracker t, Node base) {
+  t.start() and
+  isPrototypePollutingAssignment(base, _, _, _) and
+  result = base.getALocalSource()
+  or
+  exists(DataFlow::TypeBackTracker t2 |
+    result = getANodeLeadingToBase(t2, base).backtrack(t2, t)
+  )
+}
+
+/**
+ * Gets a data flow node leading to the base of dynamic property read leading to a
+ * prototype-polluting assignment.
+ *
+ * For example, this is the `dst` in `dst[key1][key2] = ...`.
+ * This dynamic read is where the reference to a built-in prototype object is obtained,
+ * and we need this to ensure that this object actually has a prototype.
+ */
+private DataFlow::SourceNode getANodeLeadingToBaseBase(DataFlow::TypeBackTracker t, Node base) {
+  exists(DynamicPropRead read |
+    read = getANodeLeadingToBase(t, base) and
+    result = read.getBase().getALocalSource()
+  )
+  or
+  exists(DataFlow::TypeBackTracker t2 |
+    result = getANodeLeadingToBaseBase(t2, base).backtrack(t2, t)
+  )
+}
+
+DataFlow::SourceNode getANodeLeadingToBaseBase(Node base) {
+  result = getANodeLeadingToBaseBase(DataFlow::TypeBackTracker::end(), base)
+}
+
+/** A call to `Object.create(null)`. */
+class ObjectCreateNullCall extends CallNode {
+  ObjectCreateNullCall() {
+    this = globalVarRef("Object").getAMemberCall("create") and
+    getArgument(0).asExpr() instanceof NullLiteral
+  }
+}
+
+from
+  PropNameTracking cfg, DataFlow::PathNode source, DataFlow::PathNode sink, EnumeratedPropName enum,
+  Node base
+where
+  cfg.hasFlowPath(source, sink) and
+  isPrototypePollutingAssignment(base, _, _, enum) and
+  sink.getNode() = base and
+  source.getNode() = enum and
+  (
+    getANodeLeadingToBaseBase(base) instanceof ObjectLiteralNode
+    or
+    not getANodeLeadingToBaseBase(base) instanceof ObjectCreateNullCall
+  )
+select base, source, sink,
+  "Properties are copied from $@ to $@ without guarding against prototype pollution.",
+  enum.getSourceObject(), deriveExprName(enum.getSourceObject()), base, deriveExprName(base)

javascript/ql/src/Security/CWE-400/examples/PrototypePollutionUtility.js

@@ -0,0 +1,10 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}

javascript/ql/src/Security/CWE-400/examples/PrototypePollutionUtility_fixed.js

@@ -0,0 +1,10 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (dst.hasOwnProperty(key) && isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}

javascript/ql/src/Security/CWE-400/examples/PrototypePollutionUtility_fixed2.js

@@ -0,0 +1,11 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (key === "__proto__" || key === "constructor") continue;
+        if (isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}

javascript/ql/src/semmle/javascript/GlobalAccessPaths.qll

@@ -3,6 +3,7 @@
  */
 
 import javascript
+private import semmle.javascript.dataflow.InferredTypes
 
 deprecated
 module GlobalAccessPath {
@@ -34,6 +35,24 @@ module GlobalAccessPath {
   }
 }
 
+/**
+ * Provides predicates for associating access paths with data flow nodes.
+ *
+ * For example, `AccessPath.getAReferenceTo(x)` can be used to obtain the global access path
+ * that `x` refers to, as in the following sample:
+ * ```
+ * function f() {
+ *   let v = foo.bar; // reference to 'foo.bar'
+ *   v.baz;           // reference to 'foo.bar.baz'
+ * }
+ *
+ * (function(ns) {
+ *   ns.x;            // reference to 'NS.x'
+ * })(NS = NS || {});
+ * ```
+ *
+ * A pseudo-property named `[number]` is sometimes used to represent array indices within an access path.
+ */
 module AccessPath {
   /**
    * A source node that can be the root of an access path.
@@ -80,6 +99,35 @@ module AccessPath {
     result = base + "." + prop
   }
 
+  /**
+   * Holds if `variable` is compared to the `length` property of something, indicating
+   * that, if used as a dynamic property name, it represents an array index.
+   */
+  private predicate isLikelyArrayIndex(SsaVariable variable) {
+    exists(RelationalComparison cmp, DataFlow::PropRead length, Expr lengthUse |
+      length.getPropertyName() = "length" and
+      length.flowsToExpr(lengthUse) and
+      cmp.hasOperands(variable.getAUse(), lengthUse)
+    )
+    or
+    isLikelyArrayIndex(variable.getDefinition().(SsaRefinementNode).getAnInput())
+  }
+
+  /**
+   * Holds if `prop` likely accesses a non-constant array element.
+   */
+  private predicate isLikelyDynamicArrayAccess(DataFlow::PropRead prop) {
+    // The implicit PropRead in a for-of loop is represented by its lvalue node
+    prop = DataFlow::lvalueNode(any(ForOfStmt stmt).getLValue())
+    or
+    // Match an index access x[i] where `i` is likely an array index variable.
+    not exists(prop.getPropertyName()) and
+    exists(SsaVariable indexVar |
+      isLikelyArrayIndex(indexVar) and
+      prop.getPropertyNameExpr() = indexVar.getAUse()
+    )
+  }
+
   /**
    * Gets the access path relative to `root` referred to by `node`.
    *
@@ -115,6 +163,9 @@ module AccessPath {
     not node.accessesGlobal(_) and
     exists(DataFlow::PropRead prop | node = prop |
       result = join(fromReference(prop.getBase(), root), prop.getPropertyName())
+      or
+      isLikelyDynamicArrayAccess(prop) and
+      result = join(fromReference(prop.getBase(), root), "[number]")
     )
     or
     exists(Closure::ClosureNamespaceAccess acc | node = acc |

javascript/ql/test/query-tests/Security/CWE-400/PrototypePollutionUtility.qlref

@@ -0,0 +1 @@
+Security/CWE-400/PrototypePollutionUtility.ql

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

@@ -0,0 +1,362 @@
+import dummy from 'somewhere';
+
+function copyUsingForIn(dst, src) {
+    for (let key in src) {
+        if (dst[key]) {
+            copyUsingForIn(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+function copyUsingKeys(dst, src) {
+    Object.keys(src).forEach(key => {
+        if (dst[key]) {
+            copyUsingKeys(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    });
+}
+
+function copyRest(dst, ...sources) {
+    for (let source of sources) {
+        for (let key in source) {
+            copyRestAux(dst, source[key], key);
+        }
+    }
+}
+
+function copyRestAux(dst, value, key) {
+    let dstValue = dst[key];
+    if (dstValue) {
+        copyRest(dstValue, value);
+    } else {
+        dst[key] = value; // NOT OK
+    }
+}
+
+function copyProtoGuarded(dst, src) {
+    for (let key in src) {
+        if (key === "__proto__") continue;
+        if (dst[key]) {
+            copyProtoGuarded(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+function copyCtorGuarded(dst, src) {
+    for (let key in src) {
+        if (key === "constructor") continue;
+        if (dst[key]) {
+            copyCtorGuarded(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+function copyDoubleGuarded(dst, src) {
+    for (let key in src) {
+        if (key === "constructor" || key === "__proto__") continue;
+        if (dst[key]) {
+            copyDoubleGuarded(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // OK
+        }
+    }
+}
+
+function isSafe(key) {
+    return key !== "__proto__" && key !== "constructor" && key !== "prototype";
+}
+
+function copyComplex(dst, src) {
+    for (let key in src) {
+        if (isSafe(key)) {
+            if (dst[key]) {
+                copyComplex(dst[key], src[key]);
+            } else {
+                dst[key] = src[key]; // OK
+            }
+        }
+    }
+}
+
+function copyHasOwnProperty(dst, src) {
+    for (let key in src) {
+        // Guarding the recursive case by dst.hasOwnProperty is safe,
+        // since '__proto__' and 'constructor' are not own properties of the destination object.
+        if (dst.hasOwnProperty(key)) {
+            copyHasOwnProperty(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // OK
+        }
+    }
+}
+
+function copyHasOwnPropertyBad(dst, src) {
+    for (let key in src) {
+        // Guarding using src.hasOwnProperty is *not* effective,
+        // since '__proto__' and 'constructor' are own properties in the payload.
+        if (!src.hasOwnProperty(key)) continue; // Not safe
+        if (dst[key]) {
+            copyHasOwnPropertyBad(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+let _hasOwnProp = Object.prototype.hasOwnProperty;
+
+function copyHasOwnPropertyTearOff(dst, src) {
+    for (let key in src) {
+        if (_hasOwnProp.call(dst, key)) {
+            copyHasOwnPropertyTearOff(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // OK
+        }
+    }
+}
+
+function shallowExtend(dst, src) {
+    for (let key in src) {
+        dst[key] = src[key]; // OK
+    }
+}
+
+function transform(src, fn) {
+    if (typeof src !== 'object') return fn(src);
+    for (let key in src) {
+        src[key] = transform(src[key], fn); // OK
+    }
+    return src;
+}
+
+function clone(src) {
+    if (typeof src !== 'object') return src;
+    let result = {};
+    for (let key in src) {
+        result[key] = clone(src[key]); // OK
+    }
+    return result;
+}
+
+function higherOrderRecursion(dst, src, callback) {
+    for (let key in src) {
+        if (dst[key]) {
+            callback(dst, src, key);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+function higherOrderRecursionEntry(dst, src) {
+    higherOrderRecursion(dst, src, (dst, src, key) => {
+        higherOrderRecursionEntry(dst[key], src[key]);
+    });
+}
+
+function instanceofObjectGuard(dst, src) {
+    for (let key in src) {
+        let dstValue = dst[key];
+        if (typeof dstValue === 'object' && dstValue instanceof Object) {
+            instanceofObjectGuard(dstValue, src[key]);
+        } else {
+            dst[key] = src[key]; // OK
+        }
+    }
+}
+
+let blacklist = ["__proto__", "constructor"];
+
+function copyWithBlacklist(dst, src) {
+    for (let key in src) {
+        if (blacklist.indexOf(key) >= 0) continue;
+        if (dst[key]) {
+            copyWithBlacklist(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // OK
+        }
+    }
+}
+
+function copyUsingPlainForLoop(dst, src) {
+    let keys = Object.keys(src);
+    for (let i = 0; i < keys.length; ++i) {
+        let key = keys[i];
+        if (dst[key]) {
+            copyUsingPlainForLoop(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    }
+}
+
+function copyUsingPlainForLoopNoAlias(dst, src) {
+    // Like copyUsingPlainForLoop, but with keys[i] duplicated at every use-site
+    let keys = Object.keys(src);
+    for (let i = 0; i < keys.length; ++i) {
+        if (dst[key]) {
+            copyUsingPlainForLoopNoAlias(dst[keys[i]], src[keys[i]]);
+        } else {
+            dst[keys[i]] = src[keys[i]]; // NOT OK - but not flagged
+        }
+    }
+}
+
+function deepSet(map, key1, key2, value) {
+    if (!map[key1]) {
+        map[key1] = Object.create(null);
+    }
+    map[key1][key2] = value; // OK
+}
+
+function deepSetCaller(data) {
+    let map1 = Object.create(null);
+    let map2 = Object.create(null);
+    for (let key in data) {
+        deepSet(map1, key, 'x', data[key]);
+        deepSet(map2, 'x', key, data[key]);
+    }
+}
+
+function deepSetBad(map, key1, key2, value) {
+    if (!map[key1]) {
+        map[key1] = Object.create(null);
+    }
+    map[key1][key2] = value; // NOT OK - object literal can flow here
+}
+
+function deepSetCallerBad(data) {
+    let map1 = Object.create(null);
+    for (let key in data) {
+        deepSetBad({}, key, 'x', data[key]); // oops
+        deepSetBad(map1, 'x', key, data[key]);
+    }
+}
+
+function maybeCopy(x) {
+    if (x && typeof x === 'object') {
+        return {...x};
+    } else {
+        return x;
+    }
+}
+
+function mergeWithCopy(dst, src) {
+    if (dst == null) return src;
+    let result = maybeCopy(dst);
+    for (let key in src) {
+        if (src.hasOwnProperty(key)) {
+            result[key] = mergeWithCopy(dst[key], src[key]); // OK
+        }
+    }
+    return result;
+}
+
+function copyUsingEntries(dst, src) {
+    Object.entries(src).forEach(entry => {
+        let key = entry[0];
+        let value = entry[1];
+        if (dst[key]) {
+            copyUsingEntries(dst[key], value);
+        } else {
+            dst[key] = value; // NOT OK
+        }
+    });
+}
+
+function copyUsingReflect(dst, src) {
+    Reflect.ownKeys(src).forEach(key => {
+        if (dst[key]) {
+            copyUsingReflect(dst[key], src[key]);
+        } else {
+            dst[key] = src[key]; // NOT OK
+        }
+    });
+}
+
+function copyWithPath(dst, src, path) {
+    for (let key in src) {
+        if (src.hasOwnProperty(key)) {
+            if (dst[key]) {
+                copyWithPath(dst[key], src[key], path ? path + '.' + key : key);
+            } else {
+                let target = {};
+                target[path] = {};
+                target[path][key] = src[key]; // OK
+                doSomething(target);
+            }
+        }
+    }
+    return dst;
+}
+
+function typeofObjectTest(dst, src) {
+    for (let key in src) {
+        if (src.hasOwnProperty(key)) {
+            let value = src[key];
+            if (dst[key] && typeof value === 'object') {
+                typeofObjectTest(dst[key], value);
+            } else {
+                dst[key] = value; // NOT OK
+            }
+        }
+    }
+}
+
+function mergeRephinementNode(dst, src) {
+    for (let key in src) {
+        if (src.hasOwnProperty(key)) {
+            if (key === key && key === key) continue; // Create a phi-node of refinement nodes
+            let value = src[key];
+            if (dst[key] && typeof value === 'object') {
+                mergeRephinementNode(dst[key], value);
+            } else {
+                dst[key] = value; // NOT OK
+            }
+        }
+    }
+}
+
+function mergeSelective(dst, src) {
+    for (let key in src) {
+        if (src.hasOwnProperty(key)) {
+            // Only 'prefs' is merged recursively
+            if (key in dst && key !== 'prefs') {
+                continue;
+            }
+            if (dst[key]) {
+                mergeSelective(dst[key], src[key]);
+            } else {
+                dst[key] = src[key]; // OK
+            }
+        }
+    }
+}
+
+function isNonArrayObject(item) {
+    return item && typeof item === 'object' && !Array.isArray(item);
+}
+
+function mergePlainObjectsOnly(target, source) {
+    if (isNonArrayObject(target) && isNonArrayObject(source)) {
+        Object.keys(source).forEach(key => {
+            if (key === '__proto__') {
+                return;
+            }
+            if (isNonArrayObject(source[key]) && key in target) {
+                target[key] = mergePlainObjectsOnly(target[key], source[key], options);
+            } else {
+                target[key] = source[key]; // OK
+            }
+        });
+    }
+    return target;
+}

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

@@ -0,0 +1,10 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}

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

@@ -0,0 +1,10 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (dst.hasOwnProperty(key) && isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}

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

@@ -0,0 +1,11 @@
+function merge(dst, src) {
+    for (let key in src) {
+        if (!src.hasOwnProperty(key)) continue;
+        if (key === "__proto__" || key === "constructor") continue;
+        if (isObject(dst[key])) {
+            merge(dst[key], src[key]);
+        } else {
+            dst[key] = src[key];
+        }
+    }
+}