github · aschackmull · Sep 27, 2021 · Jun 14, 2021 · Jun 14, 2021 · Jun 14, 2021
@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The query "XSLT transformation with user-controlled stylesheet" (`java/xslt-injection`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @ggolawski](https://github.com/github/codeql/pull/3363).
@@ -110,6 +110,7 @@ private module Frameworks {
   private import semmle.code.java.security.MvelInjection
   private import semmle.code.java.security.OgnlInjection
   private import semmle.code.java.security.XPath
+  private import semmle.code.java.security.XsltInjection
   private import semmle.code.java.frameworks.android.Android
   private import semmle.code.java.frameworks.android.SQLite
   private import semmle.code.java.frameworks.Jdbc

@@ -0,0 +1,248 @@
+/** Provides classes to reason about XSLT injection vulnerabilities. */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.ExternalFlow
+
+/**
+ * A data flow sink for unvalidated user input that is used in XSLT transformation.
+ * Extend this class to add your own XSLT Injection sinks.
+ */
+abstract class XsltInjectionSink extends DataFlow::Node { }
+
+/** A default sink representing methods susceptible to XSLT Injection attacks. */
+private class DefaultXsltInjectionSink extends XsltInjectionSink {
+  DefaultXsltInjectionSink() { sinkNode(this, "xslt") }
+}
+
+private class DefaultXsltInjectionSinkModel extends SinkModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "javax.xml.transform;Transformer;false;transform;;;Argument[-1];xslt",
+        "net.sf.saxon.s9api;XsltTransformer;false;transform;;;Argument[-1];xslt",
+        "net.sf.saxon.s9api;Xslt30Transformer;false;transform;;;Argument[-1];xslt",
+        "net.sf.saxon.s9api;Xslt30Transformer;false;applyTemplates;;;Argument[-1];xslt",
+        "net.sf.saxon.s9api;Xslt30Transformer;false;callFunction;;;Argument[-1];xslt",
+        "net.sf.saxon.s9api;Xslt30Transformer;false;callTemplate;;;Argument[-1];xslt"
+      ]
+  }
+}
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to the `XsltInjectionFlowConfig`.
+ */
+class XsltInjectionAdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for the `XsltInjectionFlowConfig` configuration.
+   */
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+/** A set of additional taint steps to consider when taint tracking XSLT related data flows. */
+private class DefaultXsltInjectionAdditionalTaintStep extends XsltInjectionAdditionalTaintStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    xmlStreamReaderStep(node1, node2) or
+    xmlEventReaderStep(node1, node2) or
+    staxSourceStep(node1, node2) or
+    documentBuilderStep(node1, node2) or
+    domSourceStep(node1, node2) or
+    newTransformerFromTemplatesStep(node1, node2) or
+    xsltCompilerStep(node1, node2) or
+    xsltExecutableStep(node1, node2) or
+    xsltPackageStep(node1, node2)
+  }
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and
+ * `XMLStreamReader`, i.e. `XMLInputFactory.createXMLStreamReader(tainted)`.
+ */
+private predicate xmlStreamReaderStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(XmlInputFactoryStreamReader xmlStreamReader |
+    if xmlStreamReader.getMethod().getParameterType(0) instanceof TypeString
+    then n1.asExpr() = xmlStreamReader.getArgument(1)
+    else n1.asExpr() = xmlStreamReader.getArgument(0)
+  |
+    n2.asExpr() = xmlStreamReader
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and
+ * `XMLEventReader`, i.e. `XMLInputFactory.createXMLEventReader(tainted)`.
+ */
+private predicate xmlEventReaderStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(XmlInputFactoryEventReader xmlEventReader |
+    if xmlEventReader.getMethod().getParameterType(0) instanceof TypeString
+    then n1.asExpr() = xmlEventReader.getArgument(1)
+    else n1.asExpr() = xmlEventReader.getArgument(0)
+  |
+    n2.asExpr() = xmlEventReader
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `XMLStreamReader` or
+ * `XMLEventReader` and `StAXSource`, i.e. `new StAXSource(tainted)`.
+ */
+private predicate staxSourceStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeStAXSource |
+    n1.asExpr() = cc.getAnArgument() and
+    n2.asExpr() = cc
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` and `Document`,
+ * i.e. `DocumentBuilder.parse(tainted)`.
+ */
+private predicate documentBuilderStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(DocumentBuilderParse documentBuilder |
+    n1.asExpr() = documentBuilder.getArgument(0) and
+    n2.asExpr() = documentBuilder
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `Document` and `DOMSource`, i.e.
+ * `new DOMSource(tainted)`.
+ */
+private predicate domSourceStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeDOMSource |
+    n1.asExpr() = cc.getAnArgument() and
+    n2.asExpr() = cc
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `Templates` and `Transformer`,
+ * i.e. `tainted.newTransformer()`.
+ */
+private predicate newTransformerFromTemplatesStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    n1.asExpr() = ma.getQualifier() and
+    n2.asExpr() = ma and
+    m.getDeclaringType() instanceof TypeTemplates and
+    m.hasName("newTransformer")
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `Source` or `URI` and
+ * `XsltExecutable` or `XsltPackage`, i.e. `XsltCompiler.compile(tainted)` or
+ * `XsltCompiler.loadExecutablePackage(tainted)` or `XsltCompiler.compilePackage(tainted)` or
+ * `XsltCompiler.loadLibraryPackage(tainted)`.
+ */
+private predicate xsltCompilerStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    n1.asExpr() = ma.getArgument(0) and
+    n2.asExpr() = ma and
+    m.getDeclaringType() instanceof TypeXsltCompiler and
+    m.hasName(["compile", "loadExecutablePackage", "compilePackage", "loadLibraryPackage"])
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `XsltExecutable` and
+ * `XsltTransformer` or `Xslt30Transformer`, i.e. `XsltExecutable.load()` or
+ * `XsltExecutable.load30()`.
+ */
+private predicate xsltExecutableStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    n1.asExpr() = ma.getQualifier() and
+    n2.asExpr() = ma and
+    m.getDeclaringType() instanceof TypeXsltExecutable and
+    m.hasName(["load", "load30"])
+  )
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `XsltPackage` and
+ * `XsltExecutable`, i.e. `XsltPackage.link()`.
+ */
+private predicate xsltPackageStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    n1.asExpr() = ma.getQualifier() and
+    n2.asExpr() = ma and
+    m.getDeclaringType() instanceof TypeXsltPackage and
+    m.hasName("link")
+  )
+}
+
+/** The class `javax.xml.transform.stax.StAXSource`. */
+private class TypeStAXSource extends Class {
+  TypeStAXSource() { this.hasQualifiedName("javax.xml.transform.stax", "StAXSource") }
+}
+
+/** The class `javax.xml.transform.dom.DOMSource`. */
+private class TypeDOMSource extends Class {
+  TypeDOMSource() { this.hasQualifiedName("javax.xml.transform.dom", "DOMSource") }
+}
+
+/** The interface `javax.xml.transform.Templates`. */
+private class TypeTemplates extends Interface {
+  TypeTemplates() { this.hasQualifiedName("javax.xml.transform", "Templates") }
+}
+
+/** The class `net.sf.saxon.s9api.XsltCompiler`. */
+private class TypeXsltCompiler extends Class {
+  TypeXsltCompiler() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltCompiler") }
+}
+
+/** The class `net.sf.saxon.s9api.XsltExecutable`. */
+private class TypeXsltExecutable extends Class {
+  TypeXsltExecutable() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltExecutable") }
+}
+
+/** The class `net.sf.saxon.s9api.XsltPackage`. */
+private class TypeXsltPackage extends Class {
+  TypeXsltPackage() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltPackage") }
+}
+
+// XmlParsers classes
+/** A call to `DocumentBuilder.parse`. */
+private class DocumentBuilderParse extends MethodAccess {
+  DocumentBuilderParse() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof DocumentBuilder and
+      m.hasName("parse")
+    )
+  }
+}
+
+/** The class `javax.xml.parsers.DocumentBuilder`. */
+private class DocumentBuilder extends RefType {
+  DocumentBuilder() { this.hasQualifiedName("javax.xml.parsers", "DocumentBuilder") }
+}
+
+/** A call to `XMLInputFactory.createXMLStreamReader`. */
+private class XmlInputFactoryStreamReader extends MethodAccess {
+  XmlInputFactoryStreamReader() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof XmlInputFactory and
+      m.hasName("createXMLStreamReader")
+    )
+  }
+}
+
+/** A call to `XMLInputFactory.createEventReader`. */
+private class XmlInputFactoryEventReader extends MethodAccess {
+  XmlInputFactoryEventReader() {
+    exists(Method m |
+      this.getMethod() = m and
+      m.getDeclaringType() instanceof XmlInputFactory and
+      m.hasName("createXMLEventReader")
+    )
+  }
+}
+
+/** The class `javax.xml.stream.XMLInputFactory`. */
+private class XmlInputFactory extends RefType {
+  XmlInputFactory() { this.hasQualifiedName("javax.xml.stream", "XMLInputFactory") }
+}
@@ -0,0 +1,90 @@
+/** Provides taint tracking configurations to be used in XSLT injection queries. */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import semmle.code.java.security.XmlParsers
+import semmle.code.java.security.XsltInjection
+
+/**
+ * A taint-tracking configuration for unvalidated user input that is used in XSLT transformation.
+ */
+class XsltInjectionFlowConfig extends TaintTracking::Configuration {
+  XsltInjectionFlowConfig() { this = "XsltInjectionFlowConfig" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof XsltInjectionSink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    node.getType() instanceof PrimitiveType or node.getType() instanceof BoxedType
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
+    any(XsltInjectionAdditionalTaintStep c).step(node1, node2)
+  }
+}
+
+/**
+ * A set of additional taint steps to consider when taint tracking XSLT related data flows.
+ * These steps use data flow logic themselves.
+ */
+private class DataFlowXsltInjectionAdditionalTaintStep extends XsltInjectionAdditionalTaintStep {
+  override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
+    newTransformerOrTemplatesStep(node1, node2)
+  }
+}
+
+/**
+ * Holds if `n1` to `n2` is a dataflow step that converts between `Source` and `Transformer` or
+ * `Templates`, i.e. `TransformerFactory.newTransformer(tainted)` or
+ * `TransformerFactory.newTemplates(tainted)`.
+ */
+private predicate newTransformerOrTemplatesStep(DataFlow::Node n1, DataFlow::Node n2) {
+  exists(MethodAccess ma, Method m | ma.getMethod() = m |
+    n1.asExpr() = ma.getAnArgument() and
+    n2.asExpr() = ma and
+    m.getDeclaringType() instanceof TransformerFactory and
+    m.hasName(["newTransformer", "newTemplates"]) and
+    not exists(TransformerFactoryWithSecureProcessingFeatureFlowConfig conf |
+      conf.hasFlowToExpr(ma.getQualifier())
+    )
+  )
+}
+
+/**
+ * A data flow configuration for secure processing feature that is enabled on `TransformerFactory`.
+ */
+private class TransformerFactoryWithSecureProcessingFeatureFlowConfig extends DataFlow2::Configuration {
+  TransformerFactoryWithSecureProcessingFeatureFlowConfig() {
+    this = "TransformerFactoryWithSecureProcessingFeatureFlowConfig"
+  }
+
+  override predicate isSource(DataFlow::Node src) {
+    exists(Variable v | v = src.asExpr().(VarAccess).getVariable() |
+      exists(TransformerFactoryFeatureConfig config | config.getQualifier() = v.getAnAccess() |
+        config.enables(configSecureProcessing())
+      )
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) {
+    exists(MethodAccess ma |
+      sink.asExpr() = ma.getQualifier() and
+      ma.getMethod().getDeclaringType() instanceof TransformerFactory
+    )
+  }
+
+  override int fieldFlowBranchLimit() { result = 0 }
+}
+
+/** A `ParserConfig` specific to `TransformerFactory`. */
+private class TransformerFactoryFeatureConfig extends ParserConfig {
+  TransformerFactoryFeatureConfig() {
+    exists(Method m |
+      m = this.getMethod() and
+      m.getDeclaringType() instanceof TransformerFactory and
+      m.hasName("setFeature")
+    )
+  }
+}
@@ -4,20 +4,20 @@
 <qhelp>
 <overview>
 <p>XSLT (Extensible Stylesheet Language Transformations) is a language for transforming XML
-documents into other XML documents or other formats. Processing of unvalidated XSLT stylesheet can
-let attacker to read arbitrary files from the filesystem or to execute arbitrary code.</p>
+documents into other XML documents or other formats. Processing unvalidated XSLT stylesheets can
+allow attackers to read arbitrary files from the filesystem or to execute arbitrary code.</p>
 </overview>
 
 <recommendation>
-<p>The general recommendation is to not process untrusted XSLT stylesheets. If user provided
+<p>The general recommendation is to not process untrusted XSLT stylesheets. If user-provided
 stylesheets must be processed, enable the secure processing mode.</p>
 </recommendation>
 
 <example>
 <p>In the following examples, the code accepts an XSLT stylesheet from the user and processes it.
 </p>
 
-<p>In the first example, the user provided XSLT stylesheet is parsed and processed.</p>
+<p>In the first example, the user-provided XSLT stylesheet is parsed and processed.</p>
 
 <p>In the second example, secure processing mode is enabled.</p>
 

@@ -1,6 +1,6 @@
 /**
  * @name XSLT transformation with user-controlled stylesheet
- * @description Doing an XSLT transformation with user-controlled stylesheet can lead to
+ * @description Performing an XSLT transformation with user-controlled stylesheets can lead to
  *              information disclosure or execution of arbitrary code.
  * @kind path-problem
  * @problem.severity error
@@ -11,8 +11,7 @@
  */
 
 import java
-import semmle.code.java.dataflow.FlowSources
-import XsltInjectionLib
+import semmle.code.java.security.XsltInjectionQuery
 import DataFlow::PathGraph
 
 from DataFlow::PathNode source, DataFlow::PathNode sink, XsltInjectionFlowConfig conf
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		lgtm,codescanning
		* The query "XSLT transformation with user-controlled stylesheet" (`java/xslt-injection`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @ggolawski](https://github.com/github/codeql/pull/3363).