JS: Dont use data label in taint-tracking configs

Author: asgerf

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

@@ -666,7 +666,12 @@ private predicate exploratoryFlowStep(
  */
 private predicate isSource(DataFlow::Node nd, DataFlow::Configuration cfg, FlowLabel lbl) {
   (cfg.isSource(nd) or nd.(AdditionalSource).isSourceFor(cfg)) and
-  lbl = FlowLabel::data()
+  (
+    if cfg instanceof TaintTracking::Configuration then
+      lbl = FlowLabel::taint()
+    else
+      lbl = FlowLabel::data()
+  )
   or
   nd.(AdditionalSource).isSourceFor(cfg, lbl)
   or
@@ -678,7 +683,12 @@ private predicate isSource(DataFlow::Node nd, DataFlow::Configuration cfg, FlowL
  */
 private predicate isSink(DataFlow::Node nd, DataFlow::Configuration cfg, FlowLabel lbl) {
   (cfg.isSink(nd) or nd.(AdditionalSink).isSinkFor(cfg)) and
-  lbl = any(StandardFlowLabel f)
+  (
+    if cfg instanceof TaintTracking::Configuration then
+      lbl = FlowLabel::taint()
+    else
+      lbl = FlowLabel::data()
+  )
   or
   nd.(AdditionalSink).isSinkFor(cfg, lbl)
   or

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

@@ -87,22 +87,26 @@ module TaintTracking {
      */
     predicate isSanitizerGuard(SanitizerGuardNode guard) { none() }
 
-    final override predicate isBarrier(DataFlow::Node node) {
-      super.isBarrier(node) or
-      isSanitizer(node) or
-      node instanceof DataFlow::VarAccessBarrier
+    override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
+      super.isLabeledBarrier(node, lbl)
+      or
+      isSanitizer(node) and lbl.isTaint()
     }
 
-    final override predicate isBarrierEdge(DataFlow::Node source, DataFlow::Node sink) {
-      super.isBarrierEdge(source, sink) or
-      isSanitizerEdge(source, sink)
+    override predicate isBarrier(DataFlow::Node node) {
+      super.isBarrier(node) or
+
+      // For variable accesses we block both the data and taint label, as a falsy value
+      // can't be an object, and thus can't have any tainted properties.
+      node instanceof DataFlow::VarAccessBarrier
     }
 
     final override predicate isBarrierEdge(
       DataFlow::Node source, DataFlow::Node sink, DataFlow::FlowLabel lbl
     ) {
       super.isBarrierEdge(source, sink, lbl) or
-      isSanitizerEdge(source, sink, lbl)
+      isSanitizerEdge(source, sink, lbl) or
+      isSanitizerEdge(source, sink) and lbl.isTaint()
     }
 
     final override predicate isBarrierGuard(DataFlow::BarrierGuardNode guard) {
@@ -157,7 +161,7 @@ module TaintTracking {
    * them.
    */
   abstract class SanitizerGuardNode extends DataFlow::BarrierGuardNode {
-    override predicate blocks(boolean outcome, Expr e) { sanitizes(outcome, e) }
+    override predicate blocks(boolean outcome, Expr e) { none() }
 
     /**
      * Holds if this node sanitizes expression `e`, provided it evaluates
@@ -166,6 +170,8 @@ module TaintTracking {
     abstract predicate sanitizes(boolean outcome, Expr e);
 
     override predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+      sanitizes(outcome, e) and label.isTaint()
+      or
       sanitizes(outcome, e, label)
     }
 

javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected

@@ -73,6 +73,7 @@ typeInferenceMismatch
 | nested-props.js:43:13:43:20 | source() | nested-props.js:44:10:44:18 | id(obj).x |
 | nested-props.js:67:31:67:38 | source() | nested-props.js:68:10:68:10 | x |
 | nested-props.js:77:36:77:43 | source() | nested-props.js:78:10:78:10 | x |
+| object-bypass-sanitizer.js:13:13:13:20 | source() | object-bypass-sanitizer.js:6:14:6:18 | x.foo |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:17:14:17:14 | x |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:20:14:20:14 | y |
 | partialCalls.js:4:17:4:24 | source() | partialCalls.js:30:14:30:20 | x.value |