JS: Move ExceptionXss source into Xss.qll

Author: asgerf

javascript/ql/src/Security/CWE-079/ExceptionXss.ql

@@ -15,18 +15,8 @@ import javascript
 import semmle.javascript.security.dataflow.ExceptionXss::ExceptionXss
 import DataFlow::PathGraph
 
-/**
- * Gets a description of the source.
- */
-string getSourceDescription(DataFlow::Node source) {
-  result = source.(ErrorSource).getDescription()
-  or
-  not source instanceof ErrorSource and
-  result = "Exception text"
-}
-
 from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
 where cfg.hasFlowPath(source, sink)
 select sink.getNode(), source, sink,
   "$@ is reinterpreted as HTML without escaping meta-characters.", source.getNode(),
-  getSourceDescription(source.getNode())
+  source.getNode().(Source).getDescription()

javascript/ql/src/semmle/javascript/security/dataflow/ExceptionXss.qll

@@ -10,6 +10,7 @@ module ExceptionXss {
   import DomBasedXssCustomizations::DomBasedXss as DomBasedXssCustom
   import ReflectedXssCustomizations::ReflectedXss as ReflectedXssCustom
   import Xss as Xss
+  import Xss::ExceptionXss
   private import semmle.javascript.dataflow.InferredTypes
 
   /**
@@ -71,14 +72,9 @@ module ExceptionXss {
     )
   }
 
-  /**
-   * A FlowLabel representing tainted data that has not been thrown in an exception.
-   * In the js/xss-through-exception query data-flow can only reach a sink after
-   * the data has been thrown as an exception, and data that has not been thrown
-   * as an exception therefore has this flow label, and only this flow label, associated with it.
-   */
-  class NotYetThrown extends DataFlow::FlowLabel {
-    NotYetThrown() { this = "NotYetThrown" }
+  // Materialize flow labels
+  private class ConcreteNotYetThrown extends Xss::ExceptionXss::NotYetThrown {
+    ConcreteNotYetThrown() { this = this }
   }
 
   /**
@@ -130,35 +126,6 @@ module ExceptionXss {
     result = getCallbackErrorParam(pred)
   }
 
-  /**
-   * A source of error values that is likely to contain unencoded user input.
-   */
-  abstract class ErrorSource extends DataFlow::Node {
-    /**
-     * Gets a human-readable description of what type of error this refers to.
-     *
-     * The result should be captialized and usable in the context of a noun.
-     */
-    abstract string getDescription();
-  }
-
-  /**
-   * An error produced by validating using `ajv`.
-   *
-   * Such an error can contain property names from the input if the
-   * underlying schema uses `additionalProperties` or `propertyPatterns`.
-   *
-   * For example, an input of form `{"<img src=x onerror=alert(1)>": 45}` might produce the error
-   * `data/<img src=x onerror=alert(1)> should be string`.
-   */
-  private class JsonSchemaValidationError extends ErrorSource {
-    JsonSchemaValidationError() {
-      this = any(JsonSchema::Ajv::Instance i).getAValidationError().getAnImmediateUse()
-    }
-
-    override string getDescription() { result = "JSON schema validation error" }
-  }
-
   /**
    * A taint-tracking configuration for reasoning about XSS with possible exceptional flow.
    * Flow labels are used to ensure that we only report taint-flow that has been thrown in
@@ -168,10 +135,7 @@ module ExceptionXss {
     Configuration() { this = "ExceptionXss" }
 
     override predicate isSource(DataFlow::Node source, DataFlow::FlowLabel label) {
-      source instanceof Xss::Shared::Source and label instanceof NotYetThrown
-      or
-      source instanceof ErrorSource and
-      label.isTaint()
+      source.(Xss::ExceptionXss::Source).getAFlowLabel() = label
     }
 
     override predicate isSink(DataFlow::Node sink, DataFlow::FlowLabel label) {

javascript/ql/src/semmle/javascript/security/dataflow/Xss.qll

@@ -602,3 +602,59 @@ module XssThroughDom {
   /** A data flow source for XSS through DOM vulnerabilities. */
   abstract class Source extends Shared::Source { }
 }
+
+/** Provides classes for customizing the `ExceptionXss` query. */
+module ExceptionXss {
+  /** A data flow source for XSS caused by interpreting exception or error text as HTML. */
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Gets a flow label to associate with this source.
+     *
+     * For sources that should pass through a `throw/catch` before reaching the sink, use the
+     * `NotYetThrown` labe. Otherwise use `taint` (the default).
+     */
+    DataFlow::FlowLabel getAFlowLabel() { result.isTaint() }
+
+    /**
+     * Gets a human-readable description of what type of error this refers to.
+     *
+     * The result should be capitalized and usable in the context of a noun.
+     */
+    string getDescription() { result = "Error text" }
+  }
+
+  /**
+   * A FlowLabel representing tainted data that has not been thrown in an exception.
+   * In the js/xss-through-exception query data-flow can only reach a sink after
+   * the data has been thrown as an exception, and data that has not been thrown
+   * as an exception therefore has this flow label, and only this flow label, associated with it.
+   */
+  abstract class NotYetThrown extends DataFlow::FlowLabel {
+    NotYetThrown() { this = "NotYetThrown" }
+  }
+
+  private class XssSourceAsSource extends Source {
+    XssSourceAsSource() { this instanceof Shared::Source }
+
+    override DataFlow::FlowLabel getAFlowLabel() { result instanceof NotYetThrown }
+
+    override string getDescription() { result = "Exception text" }
+  }
+
+  /**
+   * An error produced by validating using `ajv`.
+   *
+   * Such an error can contain property names from the input if the
+   * underlying schema uses `additionalProperties` or `propertyPatterns`.
+   *
+   * For example, an input of form `{"<img src=x onerror=alert(1)>": 45}` might produce the error
+   * `data/<img src=x onerror=alert(1)> should be string`.
+   */
+  private class JsonSchemaValidationError extends Source {
+    JsonSchemaValidationError() {
+      this = any(JsonSchema::Ajv::Instance i).getAValidationError().getAnImmediateUse()
+    }
+
+    override string getDescription() { result = "JSON schema validation error" }
+  }
+}