JavaScript: Properly handle value-preserving paths.

When constructing a path through a property write/read pair, we want to make sure that we only use value-preserving steps to track the base object. However, the value flowing in from the right-hand side of the assignment may have a different flow label (such as `taint()`), so we cannot use the normal `append` predicate to construct the composite path.

Author: Max Schaefer

javascript/ql/src/semmle/javascript/dataflow/Configuration.qll

@@ -595,8 +595,7 @@ private predicate reachableFromStoreBase(string prop, DataFlow::Node rhs, DataFl
   exists (DataFlow::Node mid, PathSummary oldSummary, PathSummary newSummary |
     reachableFromStoreBase(prop, rhs, mid, cfg, oldSummary) and
     flowStep(mid, cfg, nd, newSummary) and
-    newSummary.getEndLabel() = FlowLabel::data() and
-    summary = oldSummary.append(newSummary)
+    summary = oldSummary.appendValuePreserving(newSummary)
   )
 }
 

javascript/ql/src/semmle/javascript/dataflow/internal/FlowSteps.qll

@@ -304,6 +304,22 @@ class PathSummary extends TPathSummary {
     )
   }
 
+  /**
+   * Gets the summary for the path obtained by appending `that` to `this`, where
+   * `that` must be a path mapping `data` to `data` (in other words, it must be
+   * a value-preserving path).
+   */
+  PathSummary appendValuePreserving(PathSummary that) {
+    exists (Boolean hasReturn2, Boolean hasCall2 |
+      that = MkPathSummary(hasReturn2, hasCall2, FlowLabel::data(), FlowLabel::data()) |
+      result = MkPathSummary(hasReturn.booleanOr(hasReturn2),
+                             hasCall.booleanOr(hasCall2),
+                             start, end) and
+      // avoid constructing invalid paths
+      not (hasCall = true and hasReturn2 = true)
+    )
+  }
+
   /**
    * Gets the summary for the path obtained by appending `this` to `that`.
    */

javascript/ql/test/library-tests/InterProceduralFlow/TaintTracking.ql

@@ -33,4 +33,4 @@ class TestTaintTrackingConfiguration extends TaintTracking::Configuration {
 
 from TestTaintTrackingConfiguration tttc, DataFlow::Node src, DataFlow::Node snk
 where tttc.hasFlow(src, snk)
-select src, snk
+select src, snk