JS: Replace isOptionallySanitizedEdge with a node

Author: asgerf

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssATM.qll

@@ -23,18 +23,15 @@ class DomBasedXssAtmConfig extends AtmConfig {
 
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or
-    node instanceof DomBasedXss::Sanitizer
+    node instanceof DomBasedXss::Sanitizer or
+    DomBasedXss::isOptionallySanitizedNode(node)
   }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
     guard instanceof PrefixStringSanitizerActivated or
     guard instanceof QuoteGuard or
     guard instanceof ContainsHtmlGuard
   }
-
-  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
-    DomBasedXss::isOptionallySanitizedEdge(pred, succ)
-  }
 }
 
 private import semmle.javascript.security.dataflow.Xss::Shared as Shared

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/XssThroughDomATM.qll

@@ -23,7 +23,8 @@ class XssThroughDomAtmConfig extends AtmConfig {
 
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or
-    node instanceof DomBasedXss::Sanitizer
+    node instanceof DomBasedXss::Sanitizer or
+    DomBasedXss::isOptionallySanitizedNode(node)
   }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
@@ -34,10 +35,6 @@ class XssThroughDomAtmConfig extends AtmConfig {
     guard instanceof QuoteGuard or
     guard instanceof ContainsHtmlGuard
   }
-
-  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
-    DomBasedXss::isOptionallySanitizedEdge(pred, succ)
-  }
 }
 
 /**

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

@@ -290,9 +290,13 @@ module DomBasedXss {
   private class HtmlSanitizerAsSanitizer extends Sanitizer instanceof HtmlSanitizerCall { }
 
   /**
+   * DEPRECATED. Use `isOptionallySanitizedNode` instead.
+   *
    * Holds if there exists two dataflow edges to `succ`, where one edges is sanitized, and the other edge starts with `pred`.
    */
-  predicate isOptionallySanitizedEdge(DataFlow::Node pred, DataFlow::Node succ) {
+  deprecated predicate isOptionallySanitizedEdge = isOptionallySanitizedEdgeInternal/2;
+
+  private predicate isOptionallySanitizedEdgeInternal(DataFlow::Node pred, DataFlow::Node succ) {
     exists(HtmlSanitizerCall sanitizer |
       // sanitized = sanitize ? sanitizer(source) : source;
       exists(ConditionalExpr branch, Variable var, VarAccess access |
@@ -319,6 +323,17 @@ module DomBasedXss {
     )
   }
 
+  /**
+   * Holds if `node` should be considered optionally sanitized as it occurs in a branch
+   * that controls whether sanitization is enabled.
+   *
+   * For example, in `sanitized = sanitize ? sanitizer(source) : source`, the right-hand `source` expression
+   * is considered an optionally sanitized node.
+   */
+  predicate isOptionallySanitizedNode(DataFlow::Node node) {
+    isOptionallySanitizedEdgeInternal(_, node)
+  }
+
   /** A source of remote user input, considered as a flow source for DOM-based XSS. */
   class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { }
 

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

@@ -86,13 +86,9 @@ class Configuration extends TaintTracking::Configuration {
     // we assume that `.join()` calls have a prefix, and thus block the prefix label.
     node = any(DataFlow::MethodCallNode call | call.getMethodName() = "join") and
     lbl = prefixLabel()
-  }
-
-  override predicate isSanitizerEdge(
-    DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel label
-  ) {
-    isOptionallySanitizedEdge(pred, succ) and
-    label = [DataFlow::FlowLabel::taint(), prefixLabel(), TaintedUrlSuffix::label()]
+    or
+    isOptionallySanitizedNode(node) and
+    lbl = [DataFlow::FlowLabel::taint(), prefixLabel(), TaintedUrlSuffix::label()]
   }
 
   override predicate isAdditionalFlowStep(

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

@@ -31,10 +31,8 @@ class Configration extends TaintTracking::Configuration {
     node instanceof DomBasedXss::Sanitizer
     or
     node instanceof UnsafeJQueryPlugin::Sanitizer
-  }
-
-  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
-    DomBasedXss::isOptionallySanitizedEdge(pred, succ)
+    or
+    DomBasedXss::isOptionallySanitizedNode(node)
   }
 
   // override to require that there is a path without unmatched return steps

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

@@ -20,7 +20,8 @@ class Configuration extends TaintTracking::Configuration {
 
   override predicate isSanitizer(DataFlow::Node node) {
     super.isSanitizer(node) or
-    node instanceof DomBasedXss::Sanitizer
+    node instanceof DomBasedXss::Sanitizer or
+    DomBasedXss::isOptionallySanitizedNode(node)
   }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
@@ -32,10 +33,6 @@ class Configuration extends TaintTracking::Configuration {
     guard instanceof ContainsHtmlGuard
   }
 
-  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
-    DomBasedXss::isOptionallySanitizedEdge(pred, succ)
-  }
-
   override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
     succ = DataFlow::globalVarRef("URL").getAMemberCall("createObjectURL") and
     pred = succ.(DataFlow::InvokeNode).getArgument(0)