Merge pull request github#11486 from github/tiferet/boost-xss-through-dom

ATM: Boost XssThroughDOM

Author: jhelie

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

@@ -0,0 +1,88 @@
+/**
+ * For internal use only.
+ *
+ * A taint-tracking configuration for reasoning about XSS through the DOM.
+ * Defines shared code used by the XSS Through DOM boosted query.
+ */
+
+private import semmle.javascript.heuristics.SyntacticHeuristics
+private import semmle.javascript.security.dataflow.DomBasedXssCustomizations
+private import semmle.javascript.dataflow.InferredTypes
+private import semmle.javascript.security.dataflow.XssThroughDomCustomizations::XssThroughDom as XssThroughDom
+private import semmle.javascript.security.dataflow.UnsafeJQueryPluginCustomizations::UnsafeJQueryPlugin as UnsafeJQuery
+import AdaptiveThreatModeling
+
+class XssThroughDomAtmConfig extends AtmConfig {
+  XssThroughDomAtmConfig() { this = "XssThroughDomAtmConfig" }
+
+  override predicate isKnownSource(DataFlow::Node source) {
+    source instanceof XssThroughDom::Source
+  }
+
+  override EndpointType getASinkEndpointType() { result instanceof XssSinkType }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof DomBasedXss::Sanitizer
+  }
+
+  override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
+    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
+  }
+
+  override predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ) {
+    DomBasedXss::isOptionallySanitizedEdge(pred, succ)
+  }
+}
+
+/**
+ * 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;
+
+  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 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 {
+  QuoteGuard() { this = this }
+}
+
+private class ContainsHtmlGuard extends TaintTracking::SanitizerGuardNode, Shared::ContainsHtmlGuard {
+  ContainsHtmlGuard() { this = this }
+}

javascript/ql/experimental/adaptivethreatmodeling/modelbuilding/DebugResultInclusion.ql

@@ -16,6 +16,7 @@ private import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInj
 private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
 private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 private import experimental.adaptivethreatmodeling.XssATM as XssAtm
+private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
 
 string getAReasonSinkExcluded(DataFlow::Node sinkCandidate, Query query) {
   query instanceof NosqlInjectionQuery and
@@ -29,6 +30,9 @@ string getAReasonSinkExcluded(DataFlow::Node sinkCandidate, Query query) {
   or
   query instanceof XssQuery and
   result = any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
+  or
+  query instanceof XssThroughDomQuery and
+  result = any(XssThroughDomAtm::XssThroughDomAtmConfig cfg).getAReasonSinkExcluded(sinkCandidate)
 }
 
 pragma[inline]

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

@@ -14,6 +14,7 @@ private import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInj
 private import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
 private import experimental.adaptivethreatmodeling.TaintedPathATM as TaintedPathAtm
 private import experimental.adaptivethreatmodeling.XssATM as XssAtm
+private import experimental.adaptivethreatmodeling.XssThroughDomATM as XssThroughDomAtm
 
 /**
  * Gets the set of featureName-featureValue pairs for each endpoint in the training set.
@@ -214,6 +215,8 @@ DataFlow::Configuration getDataFlowCfg(Query query) {
   query instanceof TaintedPathQuery and result instanceof TaintedPathAtm::TaintedPathAtmConfig
   or
   query instanceof XssQuery and result instanceof XssAtm::DomBasedXssAtmConfig
+  or
+  query instanceof XssThroughDomQuery and result instanceof XssThroughDomAtm::XssThroughDomAtmConfig
 }
 
 // TODO: Delete this once we are no longer surfacing `hasFlowFromSource`.

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

@@ -8,6 +8,7 @@ import experimental.adaptivethreatmodeling.SqlInjectionATM as SqlInjectionAtm
 import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAtm
 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
@@ -23,6 +24,8 @@ where
     c instanceof TaintedPathAtm::TaintedPathAtmConfig
     or
     queryName = "Xss" and c instanceof XssAtm::DomBasedXssAtmConfig
+    or
+    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

@@ -8,7 +8,8 @@ newtype TQuery =
   TNosqlInjectionQuery() or
   TSqlInjectionQuery() or
   TTaintedPathQuery() or
-  TXssQuery()
+  TXssQuery() or
+  TXssThroughDomQuery()
 
 abstract class Query extends TQuery {
   abstract string getName();
@@ -31,3 +32,7 @@ class TaintedPathQuery extends Query, TTaintedPathQuery {
 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/XssThroughDomATM.ql

@@ -0,0 +1,25 @@
+/**
+ * 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 javascript
+import ATM::ResultsInfo
+import DataFlow::PathGraph
+import experimental.adaptivethreatmodeling.XssThroughDomATM
+
+from AtmConfig cfg, DataFlow::PathNode source, DataFlow::PathNode sink, float score
+where cfg.hasBoostedFlowPath(source, sink, score)
+select sink.getNode(), source, sink,
+  "(Experimental) $@ may be reinterpreted as HTML without escaping meta-characters. Identified using machine learning.",
+  source.getNode(), "DOM text", score

javascript/ql/experimental/adaptivethreatmodeling/test/endpoint_large_scale/EndpointFeatures.ql

@@ -11,6 +11,7 @@ import experimental.adaptivethreatmodeling.NosqlInjectionATM as NosqlInjectionAt
 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.EndpointFeatures as EndpointFeatures
 import extraction.NoFeaturizationRestrictionsConfig
 private import experimental.adaptivethreatmodeling.EndpointCharacteristics as EndpointCharacteristics
@@ -21,6 +22,7 @@ query predicate tokenFeatures(DataFlow::Node endpoint, string featureName, strin
     not exists(any(SqlInjectionAtm::SqlInjectionAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
     not exists(any(TaintedPathAtm::TaintedPathAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
     not exists(any(XssAtm::DomBasedXssAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
+    not exists(any(XssThroughDomAtm::XssThroughDomAtmConfig cfg).getAReasonSinkExcluded(endpoint)) or
     any(EndpointCharacteristics::IsArgumentToModeledFunctionCharacteristic characteristic)
         .appliesToEndpoint(endpoint)
   ) and