remove Xss from ATM

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

@@ -1,22 +1,22 @@
 /**
- * For internal use only.
- *
- * Defines shared code used by the XSS boosted query.
+ * Provides a taint-tracking configuration for reasoning about
+ * cross-site scripting vulnerabilities through the DOM.
+ * Is boosted by ATM.
  */
 
-private import semmle.javascript.heuristics.SyntacticHeuristics
-private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
+import javascript
 import AdaptiveThreatModeling
-import CoreKnowledge as CoreKnowledge
 import StandardEndpointFilters as StandardEndpointFilters
+private import semmle.javascript.dataflow.InferredTypes
+private import semmle.javascript.heuristics.SyntacticHeuristics
+private import semmle.javascript.security.dataflow.XssThroughDomCustomizations::XssThroughDom
+private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
+private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQuery
 
 /**
  * This module provides logic to filter candidate sinks to those which are likely XSS sinks.
  */
 module SinkEndpointFilter {
-  private import javascript
-  private import DomBasedXss
-
   /**
    * Provides a set of reasons why a given data flow node should be excluded as a sink candidate.
    *
@@ -60,10 +60,10 @@ module SinkEndpointFilter {
   }
 }
 
-class DomBasedXssAtmConfig extends AtmConfig {
-  DomBasedXssAtmConfig() { this = "DomBasedXssATMConfig" }
+class XssThroughDomAtmConfig extends ATMConfig {
+  XssThroughDomAtmConfig() { this = "XssThroughDomAtmConfig" }
 
-  override predicate isKnownSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
+  override predicate isKnownSource(DataFlow::Node source) { source instanceof Source }
 
   override predicate isKnownSink(DataFlow::Node sink) { sink instanceof DomBasedXss::Sink }
 
@@ -74,23 +74,16 @@ class DomBasedXssAtmConfig extends AtmConfig {
   override EndpointType getASinkEndpointType() { result instanceof XssSinkType }
 }
 
-/** DEPRECATED: Alias for DomBasedXssAtmConfig */
-deprecated class DomBasedXssATMConfig = DomBasedXssAtmConfig;
-
 /**
- * A taint-tracking configuration for reasoning about XSS vulnerabilities.
- *
- * This is largely a copy of the taint tracking configuration for the standard XSSThroughDom query,
- * except additional ATM sinks have been added to the `isSink` predicate.
+ * A taint-tracking configuration for reasoning about XSS through the DOM.
  */
 class Configuration extends TaintTracking::Configuration {
-  Configuration() { this = "DomBasedXssATMConfiguration" }
+  Configuration() { this = "XssThroughDomAtmConfig" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof DomBasedXss::Source }
+  override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
   override predicate isSink(DataFlow::Node sink) {
-    sink instanceof DomBasedXss::Sink or
-    any(DomBasedXssAtmConfig cfg).isEffectiveSink(sink)
+    (sink instanceof DomBasedXss::Sink or any(XssThroughDomAtmConfig cfg).isEffectiveSink(sink))
   }
 
   override predicate isSanitizer(DataFlow::Node node) {
@@ -99,7 +92,10 @@ class Configuration extends TaintTracking::Configuration {
   }
 
   override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
-    guard instanceof PrefixStringSanitizerActivated or
+    guard instanceof TypeTestGuard or
+    guard instanceof UnsafeJQuery::PropertyPresenceSanitizer or
+    guard instanceof UnsafeJQuery::NumberGuard or
+    guard instanceof PrefixStringSanitizer or
     guard instanceof QuoteGuard or
     guard instanceof ContainsHtmlGuard
   }
@@ -109,15 +105,43 @@ class Configuration extends TaintTracking::Configuration {
   }
 }
 
-private import semmle.javascript.security.dataflow.Xss::Shared as Shared
+/**
+ * A test of form `typeof x === "something"`, preventing `x` from being a string in some cases.
+ *
+ * This sanitizer helps prune infeasible paths in type-overloaded functions.
+ */
+class TypeTestGuard extends TaintTracking::SanitizerGuardNode, DataFlow::ValueNode {
+  override EqualityTest astNode;
+  Expr operand;
+  boolean polarity;
 
-private class PrefixStringSanitizerActivated extends TaintTracking::SanitizerGuardNode,
-  DomBasedXss::PrefixStringSanitizer {
-  PrefixStringSanitizerActivated() { this = this }
+  TypeTestGuard() {
+    exists(TypeofTag tag | TaintTracking::isTypeofGuard(astNode, operand, tag) |
+      // typeof x === "string" sanitizes `x` when it evaluates to false
+      tag = "string" and
+      polarity = astNode.getPolarity().booleanNot()
+      or
+      // typeof x === "object" sanitizes `x` when it evaluates to true
+      tag != "string" and
+      polarity = astNode.getPolarity()
+    )
+  }
+
+  override predicate sanitizes(boolean outcome, Expr e) {
+    polarity = outcome and
+    e = operand
+  }
 }
 
-private class PrefixStringActivated extends DataFlow::FlowLabel, DomBasedXss::PrefixString {
-  PrefixStringActivated() { this = this }
+private import semmle.javascript.security.dataflow.Xss::Shared as Shared
+
+private class PrefixStringSanitizer extends TaintTracking::SanitizerGuardNode,
+  DomBasedXss::PrefixStringSanitizer {
+  PrefixStringSanitizer() { this = this }
+}
+
+private class PrefixString extends DataFlow::FlowLabel, DomBasedXss::PrefixString {
+  PrefixString() { this = this }
 }
 
 private class QuoteGuard extends TaintTracking::SanitizerGuardNode, Shared::QuoteGuard {

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/counting/CountXss.ql

@@ -1,9 +0,0 @@
-/*
- * For internal use only.
- *
- *
- * Count the number of sinks and alerts for the `DomBasedXss` security query.
- */
-
-import semmle.javascript.security.dataflow.DomBasedXssQuery
-import CountAlertsAndSinks

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/evaluation/XssThroughDom.ql

@@ -0,0 +1,26 @@
+/**
+ * XssThroughDom.ql
+ *
+ * Version of the standard XSS through DOM query with an output relation ready to plug into the evaluation
+ * pipeline.
+ *
+ * Standard query: javascript/ql/src/Security/CWE-079/XssThroughDom.ql
+ */
+
+import semmle.javascript.security.dataflow.XssThroughDomQuery
+import EndToEndEvaluation as EndToEndEvaluation
+
+from
+  DataFlow::Configuration cfg, DataFlow::Node source, DataFlow::Node sink, string filePathSink,
+  int startLineSink, int endLineSink, int startColumnSink, int endColumnSink, string filePathSource,
+  int startLineSource, int endLineSource, int startColumnSource, int endColumnSource
+where
+  cfg instanceof Configuration and
+  cfg.hasFlow(source, sink) and
+  not EndToEndEvaluation::isFlowExcluded(source, sink) and
+  sink.hasLocationInfo(filePathSink, startLineSink, startColumnSink, endLineSink, endColumnSink) and
+  source
+      .hasLocationInfo(filePathSource, startLineSource, startColumnSource, endLineSource,
+        endColumnSource)
+select source, startLineSource, startColumnSource, endLineSource, endColumnSource, filePathSource,
+  sink, startLineSink, startColumnSink, endLineSink, endColumnSink, filePathSink

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/evaluation/XssThroughDomATM.ql

@@ -0,0 +1,28 @@
+/**
+ * XssThroughDomATM.ql
+ *
+ * Version of the boosted XSS through DOM query with an output relation ready to plug into the evaluation
+ * pipeline.
+ */
+
+import ATM::ResultsInfo
+import EndToEndEvaluation as EndToEndEvaluation
+import experimental.adaptivethreatmodeling.XssThroughDomATM
+
+from
+  DataFlow::Configuration cfg, DataFlow::Node source, DataFlow::Node sink, string filePathSink,
+  int startLineSink, int endLineSink, int startColumnSink, int endColumnSink, string filePathSource,
+  int startLineSource, int endLineSource, int startColumnSource, int endColumnSource, float score
+where
+  cfg.hasFlow(source, sink) and
+  not EndToEndEvaluation::isFlowExcluded(source, sink) and
+  not isFlowLikelyInBaseQuery(source, sink) and
+  sink.hasLocationInfo(filePathSink, startLineSink, startColumnSink, endLineSink, endColumnSink) and
+  source
+      .hasLocationInfo(filePathSource, startLineSource, startColumnSource, endLineSource,
+        endColumnSource) and
+  getScoreForFlow(source, sink) = score
+select source, startLineSource, startColumnSource, endLineSource, endColumnSource, filePathSource,
+  sink, startLineSink, startColumnSink, endLineSink, endColumnSink, filePathSink, score order by
+    score desc, startLineSource, startColumnSource, endLineSource, endColumnSource, filePathSource,
+    startLineSink, startColumnSink, endLineSink, endColumnSink, filePathSink

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/evaluation/XssThroughDomATMLite.ql

@@ -0,0 +1,30 @@
+/**
+ * XssThroughDomATMLite.ql
+ *
+ * Arbitrarily ranked version of the boosted XSS query with an output relation ready to plug into
+ * the evaluation pipeline. This is useful (a) for evaluating the performance of endpoint filters,
+ * and (b) as a baseline to compare the model against.
+ */
+
+import javascript
+import ATM::ResultsInfo
+import EndToEndEvaluation as EndToEndEvaluation
+import experimental.adaptivethreatmodeling.XssThroughDomATM
+
+from
+  DataFlow::Configuration cfg, DataFlow::Node source, DataFlow::Node sink, string filePathSink,
+  int startLineSink, int endLineSink, int startColumnSink, int endColumnSink, string filePathSource,
+  int startLineSource, int endLineSource, int startColumnSource, int endColumnSource, float score
+where
+  cfg.hasFlow(source, sink) and
+  not EndToEndEvaluation::isFlowExcluded(source, sink) and
+  not isFlowLikelyInBaseQuery(source, sink) and
+  sink.hasLocationInfo(filePathSink, startLineSink, startColumnSink, endLineSink, endColumnSink) and
+  source
+      .hasLocationInfo(filePathSource, startLineSource, startColumnSource, endLineSource,
+        endColumnSource) and
+  score = 0
+select source, startLineSource, startColumnSource, endLineSource, endColumnSource, filePathSource,
+  sink, startLineSink, startColumnSink, endLineSink, endColumnSink, filePathSink, score order by
+    score desc, startLineSource, startColumnSource, endLineSource, endColumnSource, filePathSource,
+    startLineSink, startColumnSink, endLineSink, endColumnSink, filePathSink

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/CountAlertsAndEndpoints.ql

@@ -10,11 +10,11 @@
  */
 
 import javascript
+import semmle.javascript.security.dataflow.CodeInjectionQuery as CodeInjection
 import semmle.javascript.security.dataflow.NosqlInjectionQuery as NosqlInjection
 import semmle.javascript.security.dataflow.SqlInjectionQuery as SqlInjection
 import semmle.javascript.security.dataflow.TaintedPathQuery as TaintedPath
 import semmle.javascript.security.dataflow.DomBasedXssQuery as DomBasedXss
-import semmle.javascript.security.dataflow.StoredXssQuery as StoredXss
 import semmle.javascript.security.dataflow.XssThroughDomQuery as XssThroughDom
 import evaluation.EndToEndEvaluation
 
@@ -29,7 +29,7 @@ select numAlerts(any(NosqlInjection::Configuration cfg)) as numNosqlAlerts,
   numAlerts(any(SqlInjection::Configuration cfg)) as numSqlAlerts,
   numAlerts(any(TaintedPath::Configuration cfg)) as numTaintedPathAlerts,
   numAlerts(any(DomBasedXss::Configuration cfg)) as numXssAlerts,
-  numAlerts(any(StoredXss::Configuration cfg)) as numStoredXssAlerts,
+  numAlerts(any(CodeInjection::Configuration cfg)) as numCodeInjectionAlerts,
   numAlerts(any(XssThroughDom::Configuration cfg)) as numXssThroughDomAlerts,
   count(DataFlow::Node sink |
     exists(NosqlInjection::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _))
@@ -44,8 +44,8 @@ select numAlerts(any(NosqlInjection::Configuration cfg)) as numNosqlAlerts,
     exists(DomBasedXss::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _))
   ) as numXssSinks,
   count(DataFlow::Node sink |
-    exists(StoredXss::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _))
-  ) as numStoredXssSinks,
+    exists(CodeInjection::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _))
+  ) as numCodeInjectionSinks,
   count(DataFlow::Node sink |
     exists(XssThroughDom::Configuration cfg | cfg.isSink(sink) or cfg.isSink(sink, _))
   ) as numXssThroughDomSinks

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointData.qll

@@ -17,7 +17,7 @@ import experimental.adaptivethreatmodeling.FilteringReasons
 import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionATM
 import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionATM
 import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathATM
-import experimental.adaptivethreatmodeling.XssATM as XssATM
+import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomATM
 import Labels
 import NoFeaturizationRestrictionsConfig
 import Queries
@@ -31,7 +31,7 @@ AtmConfig getAtmCfg(Query query) {
   or
   query instanceof TaintedPathQuery and result instanceof TaintedPathATM::TaintedPathAtmConfig
   or
-  query instanceof XssQuery and result instanceof XssATM::DomBasedXssAtmConfig
+  query instanceof XssThroughDomQuery and result instanceof XssThroughDomATM::XssThroughDomAtmConfig
 }
 
 /** DEPRECATED: Alias for getAtmCfg */
@@ -45,7 +45,7 @@ DataFlow::Configuration getDataFlowCfg(Query query) {
   or
   query instanceof TaintedPathQuery and result instanceof TaintedPathATM::Configuration
   or
-  query instanceof XssQuery and result instanceof XssATM::Configuration
+  query instanceof XssThroughDomQuery and result instanceof XssThroughDomATM::Configuration
 }
 
 /** Gets a known sink for the specified query. */

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/ExtractEndpointMapping.ql

@@ -4,10 +4,10 @@
  * Maps ML-powered queries to their `EndpointType` for clearer labelling while evaluating ML model during training.
  */
 
-import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionATM
 import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionATM
+import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionATM
 import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathATM
-import experimental.adaptivethreatmodeling.XssATM as XssATM
+import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomATM
 import experimental.adaptivethreatmodeling.AdaptiveThreatModeling
 
 from string queryName, AtmConfig c, EndpointType e
@@ -22,7 +22,7 @@ where
     queryName = "TaintedPath" and
     c instanceof TaintedPathATM::TaintedPathAtmConfig
     or
-    queryName = "Xss" and c instanceof XssATM::DomBasedXssAtmConfig
+    queryName = "XssThroughDom" and c instanceof XssThroughDomATM::XssThroughDomAtmConfig
   ) and
   e = c.getASinkEndpointType()
 select queryName, e.getEncoding() as label

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/extraction/Queries.qll

@@ -5,10 +5,12 @@
  */
 
 newtype TQuery =
+  TCodeInjectionQuery() or
   TNosqlInjectionQuery() or
   TSqlInjectionQuery() or
   TTaintedPathQuery() or
-  TXssQuery()
+  TXssQuery() or
+  TXssThroughDomQuery()
 
 abstract class Query extends TQuery {
   abstract string getName();
@@ -28,6 +30,6 @@ class TaintedPathQuery extends Query, TTaintedPathQuery {
   override string getName() { result = "TaintedPath" }
 }
 
-class XssQuery extends Query, TXssQuery {
-  override string getName() { result = "Xss" }
+class XssThroughDomQuery extends Query, TXssThroughDomQuery {
+  override string getName() { result = "XssThroughDom" }
 }

javascript/ql/experimental/adaptivethreatmodeling/src/CodeInjectionATM.ql

@@ -0,0 +1,29 @@
+/**
+ * For internal use only.
+ *
+ * @name Code injection (experimental)
+ * @description Interpreting unsanitized user input as code allows a malicious user arbitrary code execution.
+ * @kind path-problem
+ * @scored
+ * @problem.severity error
+ * @security-severity 9.3
+ * @id js/ml-powered/code-injection
+ * @tags experimental security external/cwe/cwe-094 external/cwe/cwe-095 external/cwe/cwe-079 external/cwe/cwe-116
+ */
+
+import experimental.adaptivethreatmodeling.CodeInjectionATM
+import ATM::ResultsInfo
+import DataFlow::PathGraph
+
+from
+  DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score,
+  string scoreString
+where
+  cfg.hasFlowPath(source, sink) and
+  not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
+  score = getScoreForFlow(source.getNode(), sink.getNode()) and
+  scoreString = getScoreStringForFlow(source.getNode(), sink.getNode())
+select sink.getNode(), source, sink,
+  "[Score = " + scoreString + "] This may be a js/code-injection result depending on $@ " +
+    getAdditionalAlertInfo(source.getNode(), sink.getNode()), source.getNode(),
+  "a user-provided value", score

javascript/ql/experimental/adaptivethreatmodeling/src/XssThroughDomATM.ql

@@ -0,0 +1,28 @@
+/**
+ * For internal use only.
+ *
+ * @name DOM text reinterpreted as HTML (experimental)
+ * @description Reinterpreting text from the DOM as HTML can lead to a cross-site scripting vulnerability.
+ * @kind path-problem
+ * @scored
+ * @problem.severity error
+ * @security-severity 6.1
+ * @id js/ml-powered/xss-through-dom
+ * @tags experimental security external/cwe/cwe-079 external/cwe/cwe-116
+ */
+
+import experimental.adaptivethreatmodeling.XssThroughDomATM
+import ATM::ResultsInfo
+import DataFlow::PathGraph
+
+from
+  DataFlow::Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score,
+  string scoreString
+where
+  cfg.hasFlowPath(source, sink) and
+  not isFlowLikelyInBaseQuery(source.getNode(), sink.getNode()) and
+  score = getScoreForFlow(source.getNode(), sink.getNode()) and
+  scoreString = getScoreStringForFlow(source.getNode(), sink.getNode())
+select sink.getNode(), source, sink,
+  "(Experimental) $@ may be reinterpreted as HTML without escaping meta-characters. Identified using machine learning.",
+  source.getNode(), "DOM text"

javascript/ql/experimental/adaptivethreatmodeling/src/codeql-suites/javascript-atm-code-scanning.qls

@@ -1,2 +1,7 @@
 - description: ATM boosted Code Scanning queries for JavaScript
 - queries: .
+- include:
+    query filename: NosqlInjectionATM.ql
+    query filename: SqlInjectionATM.ql
+    query filename: TaintedPathATM.ql
+    query filename: XssATM.ql