expand localFieldStep to use access-paths, and build access-paths in more cases

2025-12-21 19:26:31 +01:00 · 2022-09-06 09:12:18 +02:00
parent 74eb6b2b98
commit 843fce4bcd
7 changed files with 297 additions and 3 deletions
--- a/javascript/ql/lib/semmle/javascript/GlobalAccessPaths.qll
+++ b/javascript/ql/lib/semmle/javascript/GlobalAccessPaths.qll
@@ -233,6 +233,8 @@ module AccessPath {
      baseName = fromReference(write.getBase(), root)
      or
      baseName = fromRhs(write.getBase(), root)
+      or
+      baseName = fromRhs(GetLaterAccess::getLaterBaseAccess(write), root)
    )
    or
    exists(GlobalVariable var |
@@ -266,6 +268,100 @@ module AccessPath {
    )
  }

+  /** A module for computing an access to a variable that happens after a write to that same variable */
+  private module GetLaterAccess {
+    /**
+     * Gets an access to a variable that is written to in `write`, where the access is after the write.
+     *
+     * This allows `fromRhs` to compute an access path for e.g. the below example:
+     * ```JavaScript
+     * function Template(text, opts) {
+     *   opts = opts || {};
+     *   var options = {};
+     *   options.varName = taint;
+     *   this.opts = options; // this.opts.varName is now available
+     * }
+     * ```
+     */
+    pragma[noopt]
+    DataFlow::Node getLaterBaseAccess(DataFlow::PropWrite write) {
+      exists(
+        ControlFlowNode writeNode, BindingPattern access, VarRef otherAccess, Variable variable,
+        StmtContainer container
+      |
+        access = getBaseVar(write) and
+        writeNode = write.getWriteNode() and
+        access = getAnAccessInContainer(variable, container, true) and
+        variable = getARelevantVariable() and // manual magic
+        otherAccess = getAnAccessInContainer(variable, container, false) and
+        access != otherAccess and
+        result.asExpr() = otherAccess
+      |
+        exists(BasicBlock bb, int i, int j |
+          bb.getNode(i) = writeNode and
+          bb.getNode(j) = otherAccess and
+          i < j
+        )
+        or
+        otherAccess.getBasicBlock() = getASuccessorBBThatReadsVar(write) // more manual magic - outlined into a helper predicate.
+      )
+    }
+
+    /** Gets a variable ref that `write` writes a property to. */
+    VarRef getBaseVar(DataFlow::PropWrite write) {
+      result = write.getBase().asExpr()
+      or
+      exists(Assignment assign |
+        write.getBase().asExpr() = assign.getRhs() and
+        result = assign.getLhs()
+      )
+      or
+      exists(VariableDeclarator decl |
+        write.getBase().asExpr() = decl.getInit() and
+        result = decl.getBindingPattern()
+      )
+    }
+
+    /** Gets an access to `var` inside `container` where `usedInWrite` indicates whether the access is the base of a property write. */
+    private VarRef getAnAccessInContainer(Variable var, StmtContainer container, boolean usedInWrite) {
+      result.getVariable() = var and
+      result.getContainer() = container and
+      var.isLocal() and
+      if result = getBaseVar(_) then usedInWrite = true else usedInWrite = false
+    }
+
+    /** Gets a variable that is relevant for the computations in the `GetLaterAccess` module. */
+    private Variable getARelevantVariable() {
+      // The variable might be used where `getLaterBaseAccess()` is called.
+      exists(DataFlow::Node node |
+        exists(fromRhs(node, _)) and
+        node.asExpr().(VarAccess).getVariable() = result
+      ) and
+      // There is a write that writes to the variable.
+      getBaseVar(_).getVariable() = result and
+      // It's local.
+      result.isLocal() and // we skip global variables, because that turns messy quick.
+      // There is both a "write" and "read" in the same container of the variable.
+      exists(StmtContainer container |
+        exists(getAnAccessInContainer(result, container, true)) and // a "write", an access to the variable that is the base of a property reference.
+        exists(getAnAccessInContainer(result, container, false)) // a "read", an access to the variable that is not the base of a property reference.
+      )
+    }
+
+    /** Gets a basic-block that has a read of the variable that is written to by `write`, where the basicblock occurs after `start`. */
+    private ReachableBasicBlock getASuccessorBBThatReadsVar(DataFlow::PropWrite write) {
+      exists(VarAccess baseExpr, Variable var, ControlFlowNode writeNode |
+        baseExpr = getBaseVar(write) and
+        var = baseExpr.getVariable() and
+        var = getARelevantVariable() and
+        writeNode = write.getWriteNode() and
+        writeNode.getBasicBlock().(ReachableBasicBlock).strictlyDominates(result) and
+        // manual magic.
+        result = getAnAccessInContainer(getARelevantVariable(), _, false).getBasicBlock()
+      )
+    }
+  }
+
  /**
   * Gets a node that refers to the given access path relative to the given `root` node,
   * or `root` itself if the access path is empty.
--- a/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
+++ b/javascript/ql/lib/semmle/javascript/dataflow/DataFlow.qll
@@ -1692,10 +1692,10 @@ module DataFlow {
   */
  predicate localFieldStep(DataFlow::Node pred, DataFlow::Node succ) {
    exists(ClassNode cls, string prop |
-      pred = cls.getADirectSuperClass*().getAReceiverNode().getAPropertyWrite(prop).getRhs() or
+      pred = AccessPath::getAnAssignmentTo(cls.getADirectSuperClass*().getAReceiverNode(), prop) or
      pred = cls.getInstanceMethod(prop)
    |
-      succ = cls.getAReceiverNode().getAPropertyRead(prop)
+      succ = AccessPath::getAReferenceTo(cls.getAReceiverNode(), prop)
    )
  }

--- a/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected
@@ -170,6 +170,10 @@ nodes
 | lib/lib.js:277:23:277:26 | opts |
 | lib/lib.js:277:23:277:30 | opts.bla |
 | lib/lib.js:277:23:277:30 | opts.bla |
+| lib/lib.js:279:19:279:22 | opts |
+| lib/lib.js:279:19:279:26 | opts.bla |
+| lib/lib.js:281:23:281:35 | this.opts.bla |
+| lib/lib.js:281:23:281:35 | this.opts.bla |
 | lib/lib.js:307:39:307:42 | name |
 | lib/lib.js:307:39:307:42 | name |
 | lib/lib.js:308:23:308:26 | name |
@@ -504,8 +508,13 @@ edges
 | lib/lib.js:268:22:268:24 | obj | lib/lib.js:268:22:268:32 | obj.version |
 | lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
 | lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:26 | opts |
+| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
+| lib/lib.js:276:8:276:11 | opts | lib/lib.js:279:19:279:22 | opts |
 | lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
 | lib/lib.js:277:23:277:26 | opts | lib/lib.js:277:23:277:30 | opts.bla |
+| lib/lib.js:279:19:279:22 | opts | lib/lib.js:279:19:279:26 | opts.bla |
+| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
+| lib/lib.js:279:19:279:26 | opts.bla | lib/lib.js:281:23:281:35 | this.opts.bla |
 | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
 | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
 | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name |
@@ -714,6 +723,7 @@ edges
 | lib/lib.js:261:11:261:33 | "rm -rf ...  + name | lib/lib.js:257:35:257:38 | name | lib/lib.js:261:30:261:33 | name | $@ based on $@ is later used in $@. | lib/lib.js:261:11:261:33 | "rm -rf ...  + name | String concatenation | lib/lib.js:257:35:257:38 | name | library input | lib/lib.js:261:3:261:34 | cp.exec ... + name) | shell command |
 | lib/lib.js:268:10:268:32 | "rm -rf ... version | lib/lib.js:267:46:267:48 | obj | lib/lib.js:268:22:268:32 | obj.version | $@ based on $@ is later used in $@. | lib/lib.js:268:10:268:32 | "rm -rf ... version | String concatenation | lib/lib.js:267:46:267:48 | obj | library input | lib/lib.js:268:2:268:33 | cp.exec ... ersion) | shell command |
 | lib/lib.js:277:11:277:30 | "rm -rf " + opts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:277:23:277:30 | opts.bla | $@ based on $@ is later used in $@. | lib/lib.js:277:11:277:30 | "rm -rf " + opts.bla | String concatenation | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:277:3:277:31 | cp.exec ... ts.bla) | shell command |
+| lib/lib.js:281:11:281:35 | "rm -rf ... pts.bla | lib/lib.js:276:8:276:11 | opts | lib/lib.js:281:23:281:35 | this.opts.bla | $@ based on $@ is later used in $@. | lib/lib.js:281:11:281:35 | "rm -rf ... pts.bla | String concatenation | lib/lib.js:276:8:276:11 | opts | library input | lib/lib.js:281:3:281:36 | cp.exec ... ts.bla) | shell command |
 | lib/lib.js:308:11:308:26 | "rm -rf " + name | lib/lib.js:307:39:307:42 | name | lib/lib.js:308:23:308:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:308:11:308:26 | "rm -rf " + name | String concatenation | lib/lib.js:307:39:307:42 | name | library input | lib/lib.js:308:3:308:27 | cp.exec ... + name) | shell command |
 | lib/lib.js:315:10:315:25 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:315:22:315:25 | name | $@ based on $@ is later used in $@. | lib/lib.js:315:10:315:25 | "rm -rf " + name | String concatenation | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:315:2:315:26 | cp.exec ... + name) | shell command |
 | lib/lib.js:320:11:320:26 | "rm -rf " + name | lib/lib.js:314:40:314:43 | name | lib/lib.js:320:23:320:26 | name | $@ based on $@ is later used in $@. | lib/lib.js:320:11:320:26 | "rm -rf " + name | String concatenation | lib/lib.js:314:40:314:43 | name | library input | lib/lib.js:320:3:320:27 | cp.exec ... + name) | shell command |
--- a/javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js
+++ b/javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js
@@ -278,7 +278,7 @@ module.exports.Foo = class Foo {
 		this.opts = {};
 		this.opts.bla = opts.bla

-		cp.exec("rm -rf " + this.opts.bla); // NOT OK - but FN [INCONSISTENCY]
+		cp.exec("rm -rf " + this.opts.bla); // NOT OK
 	}
 }

--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/UnsafeCodeConstruction.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/UnsafeCodeConstruction.expected
@@ -15,6 +15,41 @@ nodes
 | lib/index.js:19:26:19:29 | data |
 | lib/index.js:22:7:22:10 | data |
 | lib/index.js:22:7:22:10 | data |
+| lib/index.js:41:32:41:35 | opts |
+| lib/index.js:41:32:41:35 | opts |
+| lib/index.js:42:3:42:19 | opts |
+| lib/index.js:42:10:42:13 | opts |
+| lib/index.js:42:10:42:19 | opts \|\| {} |
+| lib/index.js:44:21:44:24 | opts |
+| lib/index.js:44:21:44:32 | opts.varName |
+| lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:86:15:86:19 | taint |
+| lib/index.js:86:15:86:19 | taint |
+| lib/index.js:87:18:87:22 | taint |
+| lib/index.js:89:36:89:40 | taint |
+| lib/index.js:93:32:93:36 | taint |
+| lib/index.js:98:30:98:34 | taint |
+| lib/index.js:103:21:103:47 | this.op ... dOption |
+| lib/index.js:103:21:103:47 | this.op ... dOption |
+| lib/index.js:104:21:104:47 | this.op ... dOption |
+| lib/index.js:104:21:104:47 | this.op ... dOption |
+| lib/index.js:105:21:105:47 | this.op ... dOption |
+| lib/index.js:105:21:105:47 | this.op ... dOption |
+| lib/index.js:106:21:106:30 | this.taint |
+| lib/index.js:106:21:106:30 | this.taint |
+| lib/index.js:112:17:112:21 | taint |
+| lib/index.js:112:17:112:21 | taint |
+| lib/index.js:113:20:113:24 | taint |
+| lib/index.js:121:34:121:38 | taint |
+| lib/index.js:129:32:129:36 | taint |
+| lib/index.js:136:23:136:49 | this.op ... dOption |
+| lib/index.js:136:23:136:49 | this.op ... dOption |
+| lib/index.js:137:23:137:49 | this.op ... dOption |
+| lib/index.js:137:23:137:49 | this.op ... dOption |
+| lib/index.js:138:23:138:32 | this.taint |
+| lib/index.js:138:23:138:32 | this.taint |
 edges
 | lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data |
 | lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data |
@@ -32,8 +67,53 @@ edges
 | lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
 | lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
 | lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data |
+| lib/index.js:41:32:41:35 | opts | lib/index.js:42:10:42:13 | opts |
+| lib/index.js:41:32:41:35 | opts | lib/index.js:42:10:42:13 | opts |
+| lib/index.js:42:3:42:19 | opts | lib/index.js:44:21:44:24 | opts |
+| lib/index.js:42:10:42:13 | opts | lib/index.js:42:10:42:19 | opts \|\| {} |
+| lib/index.js:42:10:42:19 | opts \|\| {} | lib/index.js:42:3:42:19 | opts |
+| lib/index.js:44:21:44:24 | opts | lib/index.js:44:21:44:32 | opts.varName |
+| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:44:21:44:32 | opts.varName | lib/index.js:51:21:51:32 | opts.varName |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:87:18:87:22 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:87:18:87:22 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:89:36:89:40 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:89:36:89:40 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:93:32:93:36 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:93:32:93:36 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:98:30:98:34 | taint |
+| lib/index.js:86:15:86:19 | taint | lib/index.js:98:30:98:34 | taint |
+| lib/index.js:87:18:87:22 | taint | lib/index.js:106:21:106:30 | this.taint |
+| lib/index.js:87:18:87:22 | taint | lib/index.js:106:21:106:30 | this.taint |
+| lib/index.js:89:36:89:40 | taint | lib/index.js:103:21:103:47 | this.op ... dOption |
+| lib/index.js:89:36:89:40 | taint | lib/index.js:103:21:103:47 | this.op ... dOption |
+| lib/index.js:93:32:93:36 | taint | lib/index.js:104:21:104:47 | this.op ... dOption |
+| lib/index.js:93:32:93:36 | taint | lib/index.js:104:21:104:47 | this.op ... dOption |
+| lib/index.js:98:30:98:34 | taint | lib/index.js:105:21:105:47 | this.op ... dOption |
+| lib/index.js:98:30:98:34 | taint | lib/index.js:105:21:105:47 | this.op ... dOption |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:113:20:113:24 | taint |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:121:34:121:38 | taint |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
+| lib/index.js:112:17:112:21 | taint | lib/index.js:129:32:129:36 | taint |
+| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
+| lib/index.js:113:20:113:24 | taint | lib/index.js:138:23:138:32 | this.taint |
+| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
+| lib/index.js:121:34:121:38 | taint | lib/index.js:136:23:136:49 | this.op ... dOption |
+| lib/index.js:129:32:129:36 | taint | lib/index.js:137:23:137:49 | this.op ... dOption |
+| lib/index.js:129:32:129:36 | taint | lib/index.js:137:23:137:49 | this.op ... dOption |
 #select
 | lib/index.js:2:21:2:24 | data | lib/index.js:1:35:1:38 | data | lib/index.js:2:21:2:24 | data | $@ flows to here and is later $@. | lib/index.js:1:35:1:38 | data | Library input | lib/index.js:2:15:2:30 | "(" + data + ")" | interpreted as code |
 | lib/index.js:6:26:6:29 | name | lib/index.js:5:35:5:38 | name | lib/index.js:6:26:6:29 | name | $@ flows to here and is later $@. | lib/index.js:5:35:5:38 | name | Library input | lib/index.js:6:17:6:29 | "obj." + name | interpreted as code |
 | lib/index.js:14:21:14:24 | data | lib/index.js:13:38:13:41 | data | lib/index.js:14:21:14:24 | data | $@ flows to here and is later $@. | lib/index.js:13:38:13:41 | data | Library input | lib/index.js:14:15:14:30 | "(" + data + ")" | interpreted as code |
 | lib/index.js:22:7:22:10 | data | lib/index.js:19:26:19:29 | data | lib/index.js:22:7:22:10 | data | $@ flows to here and is later $@. | lib/index.js:19:26:19:29 | data | Library input | lib/index.js:25:24:25:26 | str | interpreted as code |
+| lib/index.js:51:21:51:32 | opts.varName | lib/index.js:41:32:41:35 | opts | lib/index.js:51:21:51:32 | opts.varName | $@ flows to here and is later $@. | lib/index.js:41:32:41:35 | opts | Library input | lib/index.js:51:10:51:52 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:103:21:103:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:103:21:103:47 | this.op ... dOption | $@ flows to here and is later $@. | lib/index.js:86:15:86:19 | taint | Library input | lib/index.js:103:10:103:67 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:104:21:104:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:104:21:104:47 | this.op ... dOption | $@ flows to here and is later $@. | lib/index.js:86:15:86:19 | taint | Library input | lib/index.js:104:10:104:67 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:105:21:105:47 | this.op ... dOption | lib/index.js:86:15:86:19 | taint | lib/index.js:105:21:105:47 | this.op ... dOption | $@ flows to here and is later $@. | lib/index.js:86:15:86:19 | taint | Library input | lib/index.js:105:10:105:67 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:106:21:106:30 | this.taint | lib/index.js:86:15:86:19 | taint | lib/index.js:106:21:106:30 | this.taint | $@ flows to here and is later $@. | lib/index.js:86:15:86:19 | taint | Library input | lib/index.js:106:10:106:50 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:136:23:136:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:136:23:136:49 | this.op ... dOption | $@ flows to here and is later $@. | lib/index.js:112:17:112:21 | taint | Library input | lib/index.js:136:12:136:69 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:137:23:137:49 | this.op ... dOption | lib/index.js:112:17:112:21 | taint | lib/index.js:137:23:137:49 | this.op ... dOption | $@ flows to here and is later $@. | lib/index.js:112:17:112:21 | taint | Library input | lib/index.js:137:12:137:69 | "  var  ... ing();" | interpreted as code |
+| lib/index.js:138:23:138:32 | this.taint | lib/index.js:112:17:112:21 | taint | lib/index.js:138:23:138:32 | this.taint | $@ flows to here and is later $@. | lib/index.js:112:17:112:21 | taint | Library input | lib/index.js:138:12:138:52 | "  var  ... ing();" | interpreted as code |
--- a/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/lib/index.js
+++ b/javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/lib/index.js
@@ -33,3 +33,109 @@ export function greySink(data) {
        }
    });
 }
+
+function codeIsAlive() {
+  new Template().compile();
+}
+
+export function Template(text, opts) {
+  opts = opts || {};
+  var options = {};
+  options.varName = opts.varName;
+  this.opts = options;
+}
+
+Template.prototype = {
+  compile: function () {
+    var opts = this.opts;
+    eval("  var " + opts.varName + " = something();"); // NOT OK
+  },
+  // The below are justs tests that ensure the global-access-path computations terminate.
+  pathsTerminate1: function (node, prev) {
+    node.tree = {
+      ancestor: node,
+      number: rand ? prev.tree.number + 1 : 0,
+    };
+  },
+  pathsTerminate2: function (A) {
+    try {
+      var B = A.p1;
+      var C = B.p2;
+      C.p5 = C;
+    } catch (ex) {}
+  },
+  pathsTerminate3: function (A) {
+    var x = foo();
+    while (Math.random()) {
+      x.r = x;
+    }
+  },
+  pathsTerminate4: function () {
+    var dest = foo();
+    var range = foo();
+    while (Math.random() < 0.5) {
+      range.tabstop = dest;
+      if (Math.random() < 0.5) {
+        dest.firstNonLinked = range;
+      }
+    }
+  },
+};
+
+export class AccessPathClass {
+  constructor(taint) {
+    this.taint = taint;
+
+    var options1 = {taintedOption: taint};
+    this.options1 = options1;
+
+    var options2;
+    options2 = {taintedOption: taint};
+    this.options2 = options2;
+
+    var options3;
+    options3 = {};
+    options3.taintedOption = taint;
+    this.options3 = options3;
+  }
+
+  doesTaint() {
+    eval("  var " + this.options1.taintedOption + " = something();"); // NOT OK
+    eval("  var " + this.options2.taintedOption + " = something();"); // NOT OK
+    eval("  var " + this.options3.taintedOption + " = something();"); // NOT OK
+    eval("  var " + this.taint + " = something();"); // NOT OK
+  }
+}
+
+
+export class AccessPathClassBB {
+    constructor(taint) {
+      this.taint = taint;
+  
+      var options1 = {taintedOption: taint};
+      if (Math.random() < 0.5) { console.log("foo"); }
+      this.options1 = options1;
+  
+      var options2;
+      if (Math.random() < 0.5) { console.log("foo"); }
+      options2 = {taintedOption: taint};
+      if (Math.random() < 0.5) { console.log("foo"); }
+      this.options2 = options2;
+  
+      var options3;
+      if (Math.random() < 0.5) { console.log("foo"); }
+      options3 = {};
+      if (Math.random() < 0.5) { console.log("foo"); }
+      options3.taintedOption = taint;
+      if (Math.random() < 0.5) { console.log("foo"); }
+      this.options3 = options3;
+    }
+  
+    doesTaint() {
+      eval("  var " + this.options1.taintedOption + " = something();"); // NOT OK
+      eval("  var " + this.options2.taintedOption + " = something();"); // NOT OK
+      eval("  var " + this.options3.taintedOption + " = something();"); // NOT OK
+      eval("  var " + this.taint + " = something();"); // NOT OK
+    }
+  }
+  
--- a/javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected
+++ b/javascript/ql/test/query-tests/Security/CWE-915/PrototypePollutingAssignment/PrototypePollutingAssignment.expected
@@ -248,7 +248,9 @@ edges
 | lib.js:55:15:55:21 | path[0] | lib.js:55:11:55:22 | obj[path[0]] |
 | lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
 | lib.js:59:18:59:18 | s | lib.js:61:17:61:17 | s |
+| lib.js:61:17:61:17 | s | lib.js:68:11:68:26 | path |
 | lib.js:61:17:61:17 | s | lib.js:68:18:68:26 | this.path |
+| lib.js:61:17:61:17 | s | lib.js:70:17:70:20 | path |
 | lib.js:68:11:68:26 | path | lib.js:70:17:70:20 | path |
 | lib.js:68:18:68:26 | this.path | lib.js:68:11:68:26 | path |
 | lib.js:70:17:70:20 | path | lib.js:70:17:70:23 | path[0] |