JS: Fix bug causing re-evaluation of cached barriers

Author: asgerf

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

@@ -287,6 +287,9 @@ private predicate isBarrierGuardInternal(Configuration cfg, BarrierGuardNodeInte
   guard.(AdditionalBarrierGuardNode).appliesTo(cfg)
   or
   guard.(DerivedBarrierGuardNode).appliesTo(cfg)
+  or
+  cfg instanceof TaintTracking::Configuration and
+  guard.(TaintTracking::AdditionalSanitizerGuardNode).appliesTo(cfg)
 }
 
 /**
@@ -390,6 +393,12 @@ abstract private class DerivedBarrierGuardNode extends BarrierGuardNodeInternal
   abstract predicate blocks(boolean outcome, Expr e, string label);
 }
 
+/**
+ * Barrier guards derived from `AdditionalSanitizerGuard`
+ */
+private class BarrierGuardNodeFromAdditionalSanitizerGuard extends BarrierGuardNodeInternal instanceof TaintTracking::AdditionalSanitizerGuardNode
+{ }
+
 /**
  * Holds if data flow node `guard` acts as a barrier for data flow.
  *
@@ -404,6 +413,10 @@ private predicate barrierGuardBlocksExpr(
   guard.(BarrierGuardNode).blocks(outcome, test, label)
   or
   guard.(DerivedBarrierGuardNode).blocks(outcome, test, label)
+  or
+  guard.(TaintTracking::AdditionalSanitizerGuardNode).sanitizes(outcome, test) and label = "taint"
+  or
+  guard.(TaintTracking::AdditionalSanitizerGuardNode).sanitizes(outcome, test, label)
 }
 
 /**
@@ -534,7 +547,7 @@ private predicate isBarrierEdgeRaw(Configuration cfg, DataFlow::Node pred, DataF
   cfg.isBarrierEdge(pred, succ)
   or
   exists(BarrierGuardNodeInternal guard |
-    cfg.isBarrierGuard(guard) and
+    isBarrierGuardInternal(cfg, guard) and
     barrierGuardBlocksEdge(guard, pred, succ, "")
   )
 }
@@ -564,7 +577,7 @@ private predicate isLabeledBarrierEdgeRaw(
   cfg.isBarrierEdge(pred, succ, label)
   or
   exists(BarrierGuardNodeInternal guard |
-    cfg.isBarrierGuard(guard) and
+    isBarrierGuardInternal(cfg, guard) and
     barrierGuardBlocksEdge(guard, pred, succ, label)
   )
 }

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

@@ -182,7 +182,39 @@ module TaintTracking {
    * for analysis-specific taint sanitizer guards.
    */
   cached
-  abstract class AdditionalSanitizerGuardNode extends SanitizerGuardNode {
+  abstract class AdditionalSanitizerGuardNode extends DataFlow::Node {
+    // For backwards compatibility, this contains a copy of the SanitizerGuard interface,
+    // but is does not inherit from it as that would cause re-evaluation of cached barriers.
+    /**
+     * Holds if this node blocks expression `e`, provided it evaluates to `outcome`.
+     */
+    cached
+    predicate blocks(boolean outcome, Expr e) { none() }
+
+    /**
+     * Holds if this node sanitizes expression `e`, provided it evaluates
+     * to `outcome`.
+     */
+    cached
+    abstract predicate sanitizes(boolean outcome, Expr e);
+
+    /**
+     * Holds if this node blocks expression `e` from flow of type `label`, provided it evaluates to `outcome`.
+     */
+    cached
+    predicate blocks(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+      this.sanitizes(outcome, e) and label.isTaint()
+      or
+      this.sanitizes(outcome, e, label)
+    }
+
+    /**
+     * Holds if this node sanitizes expression `e`, provided it evaluates
+     * to `outcome`.
+     */
+    cached
+    predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() }
+
     /**
      * Holds if this guard applies to the flow in `cfg`.
      */