JS: Migrate DomBasedXssQuery to FlowState

javascript/ql/lib/semmle/javascript/security/CommonFlowState.qll

@@ -8,6 +8,7 @@ private import TaintedUrlSuffixCustomizations
 private newtype TFlowState =
   TTaint() or
   TTaintedUrlSuffix() or
+  TTaintedPrefix()
 
 /**
  * A flow state indicating which part of a value is tainted.
@@ -24,6 +25,11 @@ class FlowState extends TFlowState {
    */
   predicate isTaintedUrlSuffix() { this = TTaintedUrlSuffix() }
 
+  /**
+   * Holds if this represents a string whose prefix is known to be tainted.
+   */
+  predicate isTaintedPrefix() { this = TTaintedPrefix() }
+
   /** Gets a string representation of this flow state. */
   string toString() {
     this.isTaint() and result = "taint"
@@ -56,6 +62,11 @@ module FlowState {
    */
   FlowState taintedUrlSuffix() { result.isTaintedUrlSuffix() }
 
+  /**
+   * Gets the flow state representing a string whose prefix is known to be tainted.
+   */
+  FlowState taintedPrefix() { result.isTaintedPrefix() }
+
   /** DEPRECATED. Gets the flow state corresponding to `label`. */
   deprecated FlowState fromFlowLabel(DataFlow::FlowLabel label) { result.toFlowLabel() = label }
 }

javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssCustomizations.qll

@@ -8,6 +8,7 @@ private import semmle.javascript.dataflow.InferredTypes
 
 module DomBasedXss {
   private import Xss::Shared as Shared
+  import semmle.javascript.security.CommonFlowState
 
   /** A data flow source for DOM-based XSS vulnerabilities. */
   abstract class Source extends Shared::Source { }
@@ -28,16 +29,16 @@ module DomBasedXss {
     predicate blocksExpr(boolean outcome, Expr e) { none() }
 
     /**
-     * Holds if this node acts as a barrier for `label`, blocking further flow from `e` if `this` evaluates to `outcome`.
+     * Holds if this node acts as a barrier for `state`, blocking further flow from `e` if `this` evaluates to `outcome`.
      */
-    predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) { none() }
+    predicate blocksExpr(boolean outcome, Expr e, FlowState state) { none() }
 
     /** DEPRECATED. Use `blocksExpr` instead. */
     deprecated predicate sanitizes(boolean outcome, Expr e) { this.blocksExpr(outcome, e) }
 
     /** DEPRECATED. Use `blocksExpr` instead. */
     deprecated predicate sanitizes(boolean outcome, Expr e, DataFlow::FlowLabel label) {
-      this.blocksExpr(outcome, e, label)
+      this.blocksExpr(outcome, e, FlowState::fromFlowLabel(label))
     }
   }
 
@@ -379,20 +380,20 @@ module DomBasedXss {
   /**
    * A flow-label representing tainted values where the prefix is attacker controlled.
    */
-  abstract class PrefixString extends DataFlow::FlowLabel {
+  abstract deprecated class PrefixString extends DataFlow::FlowLabel {
     PrefixString() { this = "PrefixString" }
   }
 
   /** Gets the flow-label representing tainted values where the prefix is attacker controlled. */
-  PrefixString prefixLabel() { any() }
+  deprecated PrefixString prefixLabel() { any() }
 
   /**
    * A sanitizer that blocks the `PrefixString` label when the start of the string is being tested as being of a particular prefix.
    */
   abstract class PrefixStringSanitizer extends BarrierGuard instanceof StringOps::StartsWith {
-    override predicate blocksExpr(boolean outcome, Expr e, DataFlow::FlowLabel label) {
+    override predicate blocksExpr(boolean outcome, Expr e, FlowState state) {
       e = super.getBaseString().asExpr() and
-      label = prefixLabel() and
+      state.isTaintedPrefix() and
       outcome = super.getPolarity()
     }
   }

javascript/ql/lib/semmle/javascript/security/dataflow/DomBasedXssQuery.qll

@@ -27,81 +27,83 @@ class HtmlSink extends DataFlow::Node instanceof Sink {
  * - URL sinks are only sinks when the scheme is user controlled
  * - JQuery selector sinks are sinks when the tainted value can start with `<`.
  *
- * The above is achieved using three flow labels:
+ * The above is achieved using three flow states:
  * - TaintedUrlSuffix: a URL where the attacker only controls a suffix.
  * - Taint: a tainted value where the attacker controls part of the value.
  * - PrefixLabel: a tainted value where the attacker controls the prefix
  */
 module DomBasedXssConfig implements DataFlow::StateConfigSig {
-  class FlowState = DataFlow::FlowLabel;
+  import semmle.javascript.security.CommonFlowState
 
-  predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
+  predicate isSource(DataFlow::Node source, FlowState state) {
     source instanceof Source and
-    (label.isTaint() or label = prefixLabel()) and
+    (state.isTaint() or state.isTaintedPrefix()) and
     not source = TaintedUrlSuffix::source()
     or
     source = TaintedUrlSuffix::source() and
-    label = TaintedUrlSuffix::label()
+    state.isTaintedUrlSuffix()
   }
 
-  predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
+  predicate isSink(DataFlow::Node sink, FlowState state) {
     sink instanceof HtmlSink and
-    label = [TaintedUrlSuffix::label(), prefixLabel(), DataFlow::FlowLabel::taint()]
+    (state.isTaint() or state.isTaintedPrefix() or state.isTaintedUrlSuffix())
     or
     sink instanceof JQueryHtmlOrSelectorSink and
-    label = [DataFlow::FlowLabel::taint(), prefixLabel()]
+    (state.isTaint() or state.isTaintedPrefix())
     or
     sink instanceof WriteUrlSink and
-    label = prefixLabel()
+    state.isTaintedPrefix()
   }
 
   predicate isBarrier(DataFlow::Node node) {
-    node instanceof Sanitizer or node = Shared::BarrierGuard::getABarrierNode()
+    node instanceof Sanitizer
+    or
+    node = Shared::BarrierGuard::getABarrierNode()
+    or
+    isOptionallySanitizedNode(node)
   }
 
-  predicate isBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
-    // copy all taint barrier guards to the TaintedUrlSuffix/PrefixLabel label
+  predicate isBarrier(DataFlow::Node node, FlowState state) {
+    // copy all taint barrier guards to the TaintedUrlSuffix/PrefixLabel state
     TaintTracking::defaultSanitizer(node) and
-    lbl = [TaintedUrlSuffix::label(), prefixLabel()]
+    (state.isTaintedUrlSuffix() or state.isTaintedPrefix())
     or
-    // any non-first string-concatenation leaf is a barrier for the prefix label.
+    // any non-first string-concatenation leaf is a barrier for the prefix state.
     exists(StringOps::ConcatenationRoot root |
       node = root.getALeaf() and
       not node = root.getFirstLeaf() and
-      lbl = prefixLabel()
+      state.isTaintedPrefix()
     )
     or
-    // we assume that `.join()` calls have a prefix, and thus block the prefix label.
+    // we assume that `.join()` calls have a prefix, and thus block the prefix state.
     node = any(DataFlow::MethodCallNode call | call.getMethodName() = "join") and
-    lbl = prefixLabel()
+    state.isTaintedPrefix()
     or
-    isOptionallySanitizedNode(node) and
-    lbl = [DataFlow::FlowLabel::taint(), prefixLabel(), TaintedUrlSuffix::label()]
+    TaintedUrlSuffix::isStateBarrier(node, TaintedUrlSuffix::FlowState::taintedUrlSuffix()) and
+    state.isTaintedUrlSuffix()
     or
-    TaintedUrlSuffix::isBarrier(node, lbl)
-    or
-    node = DataFlow::MakeLabeledBarrierGuard<BarrierGuard>::getABarrierNode(lbl)
+    node = DataFlow::MakeStateBarrierGuard<FlowState, BarrierGuard>::getABarrierNode(state)
   }
 
-  predicate isBarrierIn(DataFlow::Node node, DataFlow::FlowLabel label) { isSource(node, label) }
+  predicate isBarrierIn(DataFlow::Node node, FlowState state) { isSource(node, state) }
 
   predicate isAdditionalFlowStep(
-    DataFlow::Node node1, DataFlow::FlowLabel state1, DataFlow::Node node2,
-    DataFlow::FlowLabel state2
+    DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
   ) {
-    TaintedUrlSuffix::step(node1, node2, state1, state2)
+    TaintedUrlSuffix::isAdditionalFlowStep(node1, state1, node2, state2)
     or
     exists(DataFlow::Node operator |
       StringConcatenation::taintStep(node1, node2, operator, _) and
       StringConcatenation::getOperand(operator, 0).getStringValue() = "<" + any(string s) and
-      state1 = TaintedUrlSuffix::label() and
+      state1.isTaintedUrlSuffix() and
       state2.isTaint()
     )
     or
-    // steps out of taintedSuffixlabel to taint-label are also steps to prefixLabel.
-    TaintedUrlSuffix::step(node1, node2, TaintedUrlSuffix::label(), DataFlow::FlowLabel::taint()) and
-    state1 = TaintedUrlSuffix::label() and
-    state2 = prefixLabel()
+    // steps out of tainted-url-suffix to taint are also steps to tainted-prefix.
+    TaintedUrlSuffix::isAdditionalFlowStep(node1, FlowState::taintedUrlSuffix(), node2,
+      FlowState::taint()) and
+    state1.isTaintedUrlSuffix() and
+    state2.isTaintedPrefix()
     or
     // FIXME: this fails to work in the test case at jquery.js:37
     exists(DataFlow::FunctionNode callback, DataFlow::Node arg |
@@ -126,24 +128,25 @@ deprecated class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "HtmlInjection" }
 
   override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-    DomBasedXssConfig::isSource(source, label)
+    DomBasedXssConfig::isSource(source, FlowState::fromFlowLabel(label))
   }
 
   override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {
-    DomBasedXssConfig::isSink(sink, label)
+    DomBasedXssConfig::isSink(sink, FlowState::fromFlowLabel(label))
   }
 
   override predicate isSanitizer(DataFlow::Node node) { DomBasedXssConfig::isBarrier(node) }
 
   override predicate isLabeledBarrier(DataFlow::Node node, DataFlow::FlowLabel lbl) {
-    DomBasedXssConfig::isBarrier(node, lbl)
+    DomBasedXssConfig::isBarrier(node, FlowState::fromFlowLabel(lbl))
   }
 
   override predicate isAdditionalFlowStep(
     DataFlow::Node node1, DataFlow::Node node2, DataFlow::FlowLabel state1,
     DataFlow::FlowLabel state2
   ) {
-    DomBasedXssConfig::isAdditionalFlowStep(node1, state1, node2, state2)
+    DomBasedXssConfig::isAdditionalFlowStep(node1, FlowState::fromFlowLabel(state1), node2,
+      FlowState::fromFlowLabel(state2))
     or
     // inherit all ordinary taint steps for the prefix label
     state1 = prefixLabel() and
@@ -156,7 +159,7 @@ private class PrefixStringSanitizerActivated extends PrefixStringSanitizer {
   PrefixStringSanitizerActivated() { this = this }
 }
 
-private class PrefixStringActivated extends DataFlow::FlowLabel, PrefixString {
+deprecated private class PrefixStringActivated extends DataFlow::FlowLabel, PrefixString {
   PrefixStringActivated() { this = this }
 }