diff --git a/java/change-notes/2021-06-16-xslt-injection-query.md b/java/change-notes/2021-06-16-xslt-injection-query.md new file mode 100644 index 000000000000..d30421607b3c --- /dev/null +++ b/java/change-notes/2021-06-16-xslt-injection-query.md @@ -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). \ No newline at end of file diff --git a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll index bce80a3ee08a..7f46f2d79b60 100644 --- a/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll @@ -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 diff --git a/java/ql/lib/semmle/code/java/security/XsltInjection.qll b/java/ql/lib/semmle/code/java/security/XsltInjection.qll new file mode 100644 index 000000000000..752884920046 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/XsltInjection.qll @@ -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") } +} diff --git a/java/ql/lib/semmle/code/java/security/XsltInjectionQuery.qll b/java/ql/lib/semmle/code/java/security/XsltInjectionQuery.qll new file mode 100644 index 000000000000..34e533c00405 --- /dev/null +++ b/java/ql/lib/semmle/code/java/security/XsltInjectionQuery.qll @@ -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") + ) + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.java b/java/ql/src/Security/CWE/CWE-074/XsltInjection.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.java rename to java/ql/src/Security/CWE/CWE-074/XsltInjection.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.qhelp b/java/ql/src/Security/CWE/CWE-074/XsltInjection.qhelp similarity index 82% rename from java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.qhelp rename to java/ql/src/Security/CWE/CWE-074/XsltInjection.qhelp index 4392550d33fb..402f4f7f5db1 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.qhelp +++ b/java/ql/src/Security/CWE/CWE-074/XsltInjection.qhelp @@ -4,12 +4,12 @@

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.

+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.

-

The general recommendation is to not process untrusted XSLT stylesheets. If user provided +

The general recommendation is to not process untrusted XSLT stylesheets. If user-provided stylesheets must be processed, enable the secure processing mode.

@@ -17,7 +17,7 @@ stylesheets must be processed, enable the secure processing mode.

In the following examples, the code accepts an XSLT stylesheet from the user and processes it.

-

In the first example, the user provided XSLT stylesheet is parsed and processed.

+

In the first example, the user-provided XSLT stylesheet is parsed and processed.

In the second example, secure processing mode is enabled.

diff --git a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.ql b/java/ql/src/Security/CWE/CWE-074/XsltInjection.ql similarity index 78% rename from java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.ql rename to java/ql/src/Security/CWE/CWE-074/XsltInjection.ql index 1403573e4b19..aeec7441cb4c 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjection.ql +++ b/java/ql/src/Security/CWE/CWE-074/XsltInjection.ql @@ -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 diff --git a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjectionLib.qll b/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjectionLib.qll deleted file mode 100644 index 4ba0eb6d0b11..000000000000 --- a/java/ql/src/experimental/Security/CWE/CWE-074/XsltInjectionLib.qll +++ /dev/null @@ -1,288 +0,0 @@ -import java -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.security.XmlParsers -import DataFlow - -/** - * 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) { - xmlStreamReaderStep(node1, node2) or - xmlEventReaderStep(node1, node2) or - staxSourceStep(node1, node2) or - documentBuilderStep(node1, node2) or - domSourceStep(node1, node2) or - newTransformerOrTemplatesStep(node1, node2) or - newTransformerFromTemplatesStep(node1, node2) or - xsltCompilerStep(node1, node2) or - xsltExecutableStep(node1, node2) or - xsltPackageStep(node1, node2) - } -} - -/** The class `javax.xml.transform.stax.StAXSource`. */ -class TypeStAXSource extends Class { - TypeStAXSource() { this.hasQualifiedName("javax.xml.transform.stax", "StAXSource") } -} - -/** The class `javax.xml.transform.dom.DOMSource`. */ -class TypeDOMSource extends Class { - TypeDOMSource() { this.hasQualifiedName("javax.xml.transform.dom", "DOMSource") } -} - -/** The interface `javax.xml.transform.Templates`. */ -class TypeTemplates extends Interface { - TypeTemplates() { this.hasQualifiedName("javax.xml.transform", "Templates") } -} - -/** The method `net.sf.saxon.s9api.XsltTransformer.transform`. */ -class XsltTransformerTransformMethod extends Method { - XsltTransformerTransformMethod() { - this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "XsltTransformer") and - this.hasName("transform") - } -} - -/** The method `net.sf.saxon.s9api.Xslt30Transformer.transform`. */ -class Xslt30TransformerTransformMethod extends Method { - Xslt30TransformerTransformMethod() { - this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and - this.hasName("transform") - } -} - -/** The method `net.sf.saxon.s9api.Xslt30Transformer.applyTemplates`. */ -class Xslt30TransformerApplyTemplatesMethod extends Method { - Xslt30TransformerApplyTemplatesMethod() { - this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and - this.hasName("applyTemplates") - } -} - -/** The method `net.sf.saxon.s9api.Xslt30Transformer.callFunction`. */ -class Xslt30TransformerCallFunctionMethod extends Method { - Xslt30TransformerCallFunctionMethod() { - this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and - this.hasName("callFunction") - } -} - -/** The method `net.sf.saxon.s9api.Xslt30Transformer.callTemplate`. */ -class Xslt30TransformerCallTemplateMethod extends Method { - Xslt30TransformerCallTemplateMethod() { - this.getDeclaringType().hasQualifiedName("net.sf.saxon.s9api", "Xslt30Transformer") and - this.hasName("callTemplate") - } -} - -/** The class `net.sf.saxon.s9api.XsltCompiler`. */ -class TypeXsltCompiler extends Class { - TypeXsltCompiler() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltCompiler") } -} - -/** The class `net.sf.saxon.s9api.XsltExecutable`. */ -class TypeXsltExecutable extends Class { - TypeXsltExecutable() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltExecutable") } -} - -/** The class `net.sf.saxon.s9api.XsltPackage`. */ -class TypeXsltPackage extends Class { - TypeXsltPackage() { this.hasQualifiedName("net.sf.saxon.s9api", "XsltPackage") } -} - -/** A data flow sink for unvalidated user input that is used in XSLT transformation. */ -class XsltInjectionSink extends DataFlow::ExprNode { - XsltInjectionSink() { - exists(MethodAccess ma, Method m | m = ma.getMethod() and ma.getQualifier() = this.getExpr() | - ma instanceof TransformerTransform or - m instanceof XsltTransformerTransformMethod or - m instanceof Xslt30TransformerTransformMethod or - m instanceof Xslt30TransformerApplyTemplatesMethod or - m instanceof Xslt30TransformerCallFunctionMethod or - m instanceof Xslt30TransformerCallTemplateMethod - ) - } -} - -/** - * Holds if `n1` to `n2` is a dataflow step that converts between `InputStream` or `Reader` and - * `XMLStreamReader`, i.e. `XMLInputFactory.createXMLStreamReader(tainted)`. - */ -predicate xmlStreamReaderStep(ExprNode n1, ExprNode n2) { - exists(XmlInputFactoryStreamReader xmlStreamReader | - n1.asExpr() = xmlStreamReader.getSink() and - 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)`. - */ -predicate xmlEventReaderStep(ExprNode n1, ExprNode n2) { - exists(XmlInputFactoryEventReader xmlEventReader | - n1.asExpr() = xmlEventReader.getSink() and - 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)`. - */ -predicate staxSourceStep(ExprNode n1, ExprNode 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)`. - */ -predicate documentBuilderStep(ExprNode n1, ExprNode n2) { - exists(DocumentBuilderParse documentBuilder | - n1.asExpr() = documentBuilder.getSink() and - n2.asExpr() = documentBuilder - ) -} - -/** - * Holds if `n1` to `n2` is a dataflow step that converts between `Document` and `DOMSource`, i.e. - * `new DOMSource(tainted)`. - */ -predicate domSourceStep(ExprNode n1, ExprNode n2) { - exists(ConstructorCall cc | cc.getConstructedType() instanceof TypeDOMSource | - n1.asExpr() = cc.getAnArgument() and - n2.asExpr() = cc - ) -} - -/** - * 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") - ) - } -} - -/** - * 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)`. - */ -predicate newTransformerOrTemplatesStep(ExprNode n1, ExprNode 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") - or - m.getDeclaringType() instanceof TransformerFactory and m.hasName("newTemplates") - ) and - not exists(TransformerFactoryWithSecureProcessingFeatureFlowConfig conf | - conf.hasFlowToExpr(ma.getQualifier()) - ) - ) -} - -/** - * Holds if `n1` to `n2` is a dataflow step that converts between `Templates` and `Transformer`, - * i.e. `tainted.newTransformer()`. - */ -predicate newTransformerFromTemplatesStep(ExprNode n1, ExprNode 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)`. - */ -predicate xsltCompilerStep(ExprNode n1, ExprNode 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") or - m.hasName("loadExecutablePackage") or - m.hasName("compilePackage") or - m.hasName("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()`. - */ -predicate xsltExecutableStep(ExprNode n1, ExprNode 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") or m.hasName("load30")) - ) -} - -/** - * Holds if `n1` to `n2` is a dataflow step that converts between `XsltPackage` and - * `XsltExecutable`, i.e. `XsltPackage.link()`. - */ -predicate xsltPackageStep(ExprNode n1, ExprNode 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") - ) -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.expected b/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.expected deleted file mode 100644 index 99cde4c3c10e..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.expected +++ /dev/null @@ -1,122 +0,0 @@ -edges -| XsltInjection.java:30:27:30:67 | new StreamSource(...) : StreamSource | XsltInjection.java:31:5:31:59 | newTransformer(...) | -| XsltInjection.java:30:44:30:66 | getInputStream(...) : InputStream | XsltInjection.java:30:27:30:67 | new StreamSource(...) : StreamSource | -| XsltInjection.java:35:27:35:90 | new StreamSource(...) : StreamSource | XsltInjection.java:36:5:36:74 | newTransformer(...) | -| XsltInjection.java:35:44:35:89 | new InputStreamReader(...) : InputStreamReader | XsltInjection.java:35:27:35:90 | new StreamSource(...) : StreamSource | -| XsltInjection.java:35:66:35:88 | getInputStream(...) : InputStream | XsltInjection.java:35:44:35:89 | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:40:45:40:70 | param : String | XsltInjection.java:42:61:42:64 | xslt : String | -| XsltInjection.java:42:27:42:66 | new StreamSource(...) : StreamSource | XsltInjection.java:43:5:43:59 | newTransformer(...) | -| XsltInjection.java:42:44:42:65 | new StringReader(...) : StringReader | XsltInjection.java:42:27:42:66 | new StreamSource(...) : StreamSource | -| XsltInjection.java:42:61:42:64 | xslt : String | XsltInjection.java:42:44:42:65 | new StringReader(...) : StringReader | -| XsltInjection.java:47:24:47:78 | new SAXSource(...) : SAXSource | XsltInjection.java:48:5:48:74 | newTransformer(...) | -| XsltInjection.java:47:38:47:77 | new InputSource(...) : InputSource | XsltInjection.java:47:24:47:78 | new SAXSource(...) : SAXSource | -| XsltInjection.java:47:54:47:76 | getInputStream(...) : InputStream | XsltInjection.java:47:38:47:77 | new InputSource(...) : InputSource | -| XsltInjection.java:52:24:52:107 | new SAXSource(...) : SAXSource | XsltInjection.java:53:5:53:59 | newTransformer(...) | -| XsltInjection.java:52:44:52:106 | new InputSource(...) : InputSource | XsltInjection.java:52:24:52:107 | new SAXSource(...) : SAXSource | -| XsltInjection.java:52:60:52:105 | new InputStreamReader(...) : InputStreamReader | XsltInjection.java:52:44:52:106 | new InputSource(...) : InputSource | -| XsltInjection.java:52:82:52:104 | getInputStream(...) : InputStream | XsltInjection.java:52:60:52:105 | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:57:91:57:113 | getInputStream(...) : InputStream | XsltInjection.java:58:5:58:59 | newTransformer(...) | -| XsltInjection.java:62:98:62:143 | new InputStreamReader(...) : InputStreamReader | XsltInjection.java:63:5:63:74 | newTransformer(...) | -| XsltInjection.java:62:120:62:142 | getInputStream(...) : InputStream | XsltInjection.java:62:98:62:143 | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:67:102:67:124 | getInputStream(...) : InputStream | XsltInjection.java:68:5:68:59 | newTransformer(...) | -| XsltInjection.java:72:27:72:67 | new StreamSource(...) : StreamSource | XsltInjection.java:76:5:76:34 | newTransformer(...) | -| XsltInjection.java:72:44:72:66 | getInputStream(...) : InputStream | XsltInjection.java:72:27:72:67 | new StreamSource(...) : StreamSource | -| XsltInjection.java:80:27:80:67 | new StreamSource(...) : StreamSource | XsltInjection.java:83:5:83:34 | newTransformer(...) | -| XsltInjection.java:80:44:80:66 | getInputStream(...) : InputStream | XsltInjection.java:80:27:80:67 | new StreamSource(...) : StreamSource | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:90:5:90:35 | load(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:91:5:91:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:92:5:92:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:93:5:93:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:94:5:94:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:95:5:95:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:96:5:96:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:97:5:97:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:98:5:98:37 | load30(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | XsltInjection.java:99:5:99:37 | load30(...) | -| XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | -| XsltInjection.java:103:36:103:61 | param : String | XsltInjection.java:104:23:104:27 | param : String | -| XsltInjection.java:104:15:104:28 | new URI(...) : URI | XsltInjection.java:108:5:108:46 | load(...) | -| XsltInjection.java:104:15:104:28 | new URI(...) : URI | XsltInjection.java:110:5:110:50 | load(...) | -| XsltInjection.java:104:23:104:27 | param : String | XsltInjection.java:104:15:104:28 | new URI(...) : URI | -| XsltInjection.java:105:27:105:67 | new StreamSource(...) : StreamSource | XsltInjection.java:109:5:109:49 | load(...) | -| XsltInjection.java:105:44:105:66 | getInputStream(...) : InputStream | XsltInjection.java:105:27:105:67 | new StreamSource(...) : StreamSource | -nodes -| XsltInjection.java:30:27:30:67 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:30:44:30:66 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:31:5:31:59 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:35:27:35:90 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:35:44:35:89 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:35:66:35:88 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:36:5:36:74 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:40:45:40:70 | param : String | semmle.label | param : String | -| XsltInjection.java:42:27:42:66 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:42:44:42:65 | new StringReader(...) : StringReader | semmle.label | new StringReader(...) : StringReader | -| XsltInjection.java:42:61:42:64 | xslt : String | semmle.label | xslt : String | -| XsltInjection.java:43:5:43:59 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:47:24:47:78 | new SAXSource(...) : SAXSource | semmle.label | new SAXSource(...) : SAXSource | -| XsltInjection.java:47:38:47:77 | new InputSource(...) : InputSource | semmle.label | new InputSource(...) : InputSource | -| XsltInjection.java:47:54:47:76 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:48:5:48:74 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:52:24:52:107 | new SAXSource(...) : SAXSource | semmle.label | new SAXSource(...) : SAXSource | -| XsltInjection.java:52:44:52:106 | new InputSource(...) : InputSource | semmle.label | new InputSource(...) : InputSource | -| XsltInjection.java:52:60:52:105 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:52:82:52:104 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:53:5:53:59 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:57:91:57:113 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:58:5:58:59 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:62:98:62:143 | new InputStreamReader(...) : InputStreamReader | semmle.label | new InputStreamReader(...) : InputStreamReader | -| XsltInjection.java:62:120:62:142 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:63:5:63:74 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:67:102:67:124 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:68:5:68:59 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:72:27:72:67 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:72:44:72:66 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:76:5:76:34 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:80:27:80:67 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:80:44:80:66 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:83:5:83:34 | newTransformer(...) | semmle.label | newTransformer(...) | -| XsltInjection.java:87:27:87:67 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:90:5:90:35 | load(...) | semmle.label | load(...) | -| XsltInjection.java:91:5:91:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:92:5:92:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:93:5:93:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:94:5:94:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:95:5:95:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:96:5:96:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:97:5:97:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:98:5:98:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:99:5:99:37 | load30(...) | semmle.label | load30(...) | -| XsltInjection.java:103:36:103:61 | param : String | semmle.label | param : String | -| XsltInjection.java:104:15:104:28 | new URI(...) : URI | semmle.label | new URI(...) : URI | -| XsltInjection.java:104:23:104:27 | param : String | semmle.label | param : String | -| XsltInjection.java:105:27:105:67 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | -| XsltInjection.java:105:44:105:66 | getInputStream(...) : InputStream | semmle.label | getInputStream(...) : InputStream | -| XsltInjection.java:108:5:108:46 | load(...) | semmle.label | load(...) | -| XsltInjection.java:109:5:109:49 | load(...) | semmle.label | load(...) | -| XsltInjection.java:110:5:110:50 | load(...) | semmle.label | load(...) | -subpaths -#select -| XsltInjection.java:31:5:31:59 | newTransformer(...) | XsltInjection.java:30:44:30:66 | getInputStream(...) : InputStream | XsltInjection.java:31:5:31:59 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:30:44:30:66 | getInputStream(...) | this user input | -| XsltInjection.java:36:5:36:74 | newTransformer(...) | XsltInjection.java:35:66:35:88 | getInputStream(...) : InputStream | XsltInjection.java:36:5:36:74 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:35:66:35:88 | getInputStream(...) | this user input | -| XsltInjection.java:43:5:43:59 | newTransformer(...) | XsltInjection.java:40:45:40:70 | param : String | XsltInjection.java:43:5:43:59 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:40:45:40:70 | param | this user input | -| XsltInjection.java:48:5:48:74 | newTransformer(...) | XsltInjection.java:47:54:47:76 | getInputStream(...) : InputStream | XsltInjection.java:48:5:48:74 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:47:54:47:76 | getInputStream(...) | this user input | -| XsltInjection.java:53:5:53:59 | newTransformer(...) | XsltInjection.java:52:82:52:104 | getInputStream(...) : InputStream | XsltInjection.java:53:5:53:59 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:52:82:52:104 | getInputStream(...) | this user input | -| XsltInjection.java:58:5:58:59 | newTransformer(...) | XsltInjection.java:57:91:57:113 | getInputStream(...) : InputStream | XsltInjection.java:58:5:58:59 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:57:91:57:113 | getInputStream(...) | this user input | -| XsltInjection.java:63:5:63:74 | newTransformer(...) | XsltInjection.java:62:120:62:142 | getInputStream(...) : InputStream | XsltInjection.java:63:5:63:74 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:62:120:62:142 | getInputStream(...) | this user input | -| XsltInjection.java:68:5:68:59 | newTransformer(...) | XsltInjection.java:67:102:67:124 | getInputStream(...) : InputStream | XsltInjection.java:68:5:68:59 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:67:102:67:124 | getInputStream(...) | this user input | -| XsltInjection.java:76:5:76:34 | newTransformer(...) | XsltInjection.java:72:44:72:66 | getInputStream(...) : InputStream | XsltInjection.java:76:5:76:34 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:72:44:72:66 | getInputStream(...) | this user input | -| XsltInjection.java:83:5:83:34 | newTransformer(...) | XsltInjection.java:80:44:80:66 | getInputStream(...) : InputStream | XsltInjection.java:83:5:83:34 | newTransformer(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:80:44:80:66 | getInputStream(...) | this user input | -| XsltInjection.java:90:5:90:35 | load(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:90:5:90:35 | load(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:91:5:91:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:91:5:91:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:92:5:92:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:92:5:92:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:93:5:93:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:93:5:93:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:94:5:94:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:94:5:94:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:95:5:95:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:95:5:95:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:96:5:96:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:96:5:96:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:97:5:97:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:97:5:97:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:98:5:98:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:98:5:98:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:99:5:99:37 | load30(...) | XsltInjection.java:87:44:87:66 | getInputStream(...) : InputStream | XsltInjection.java:99:5:99:37 | load30(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:87:44:87:66 | getInputStream(...) | this user input | -| XsltInjection.java:108:5:108:46 | load(...) | XsltInjection.java:103:36:103:61 | param : String | XsltInjection.java:108:5:108:46 | load(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:103:36:103:61 | param | this user input | -| XsltInjection.java:109:5:109:49 | load(...) | XsltInjection.java:105:44:105:66 | getInputStream(...) : InputStream | XsltInjection.java:109:5:109:49 | load(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:105:44:105:66 | getInputStream(...) | this user input | -| XsltInjection.java:110:5:110:50 | load(...) | XsltInjection.java:103:36:103:61 | param : String | XsltInjection.java:110:5:110:50 | load(...) | XSLT transformation might include stylesheet from $@. | XsltInjection.java:103:36:103:61 | param | this user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.qlref deleted file mode 100644 index eb4c280c5df4..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-074/XsltInjection.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-074/options b/java/ql/test/experimental/query-tests/security/CWE-074/options deleted file mode 100644 index 599f2f5f14aa..000000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-074/options +++ /dev/null @@ -1 +0,0 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/Saxon-HE-9.9.1-7 diff --git a/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.expected b/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.java b/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.java similarity index 68% rename from java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.java rename to java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.java index d1a19217f02e..2bfd02a865c6 100644 --- a/java/ql/test/experimental/query-tests/security/CWE-074/XsltInjection.java +++ b/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.java @@ -25,47 +25,51 @@ import net.sf.saxon.s9api.XsltCompiler; @Controller -public class XsltInjection { +public class XsltInjectionTest { public void testStreamSourceInputStream(Socket socket) throws Exception { StreamSource source = new StreamSource(socket.getInputStream()); - TransformerFactory.newInstance().newTransformer(source).transform(null, null); + TransformerFactory.newInstance().newTransformer(source).transform(null, null); // $hasXsltInjection } public void testStreamSourceReader(Socket socket) throws Exception { StreamSource source = new StreamSource(new InputStreamReader(socket.getInputStream())); - TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); + TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); // $hasXsltInjection } @RequestMapping public void testStreamSourceInjectedParam(@RequestParam String param) throws Exception { String xslt = ""; StreamSource source = new StreamSource(new StringReader(xslt)); - TransformerFactory.newInstance().newTransformer(source).transform(null, null); + TransformerFactory.newInstance().newTransformer(source).transform(null, null); // $hasXsltInjection } public void testSAXSourceInputStream(Socket socket) throws Exception { SAXSource source = new SAXSource(new InputSource(socket.getInputStream())); - TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); + TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); // $hasXsltInjection } public void testSAXSourceReader(Socket socket) throws Exception { - SAXSource source = new SAXSource(null, new InputSource(new InputStreamReader(socket.getInputStream()))); - TransformerFactory.newInstance().newTransformer(source).transform(null, null); + SAXSource source = + new SAXSource(null, new InputSource(new InputStreamReader(socket.getInputStream()))); + TransformerFactory.newInstance().newTransformer(source).transform(null, null); // $hasXsltInjection } public void testStAXSourceEventReader(Socket socket) throws Exception { - StAXSource source = new StAXSource(XMLInputFactory.newInstance().createXMLEventReader(socket.getInputStream())); - TransformerFactory.newInstance().newTransformer(source).transform(null, null); + StAXSource source = + new StAXSource(XMLInputFactory.newInstance().createXMLEventReader(socket.getInputStream())); + TransformerFactory.newInstance().newTransformer(source).transform(null, null); // $hasXsltInjection } public void testStAXSourceEventStream(Socket socket) throws Exception { - StAXSource source = new StAXSource(XMLInputFactory.newInstance().createXMLStreamReader(null, new InputStreamReader(socket.getInputStream()))); - TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); + StAXSource source = new StAXSource(XMLInputFactory.newInstance().createXMLStreamReader(null, + new InputStreamReader(socket.getInputStream()))); + TransformerFactory.newInstance().newTemplates(source).newTransformer().transform(null, null); // $hasXsltInjection } public void testDOMSource(Socket socket) throws Exception { - DOMSource source = new DOMSource(DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(socket.getInputStream())); - TransformerFactory.newInstance().newTransformer(source).transform(null, null); + DOMSource source = new DOMSource( + DocumentBuilderFactory.newInstance().newDocumentBuilder().parse(socket.getInputStream())); + TransformerFactory.newInstance().newTransformer(source).transform(null, null); // $hasXsltInjection } public void testDisabledXXE(Socket socket) throws Exception { @@ -73,30 +77,30 @@ public void testDisabledXXE(Socket socket) throws Exception { TransformerFactory factory = TransformerFactory.newInstance(); factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, ""); factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, ""); - factory.newTransformer(source).transform(null, null); + factory.newTransformer(source).transform(null, null); // $hasXsltInjection } public void testFeatureSecureProcessingDisabled(Socket socket) throws Exception { StreamSource source = new StreamSource(socket.getInputStream()); TransformerFactory factory = TransformerFactory.newInstance(); factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, false); - factory.newTransformer(source).transform(null, null); + factory.newTransformer(source).transform(null, null); // $hasXsltInjection } public void testSaxon(Socket socket) throws Exception { StreamSource source = new StreamSource(socket.getInputStream()); XsltCompiler compiler = new Processor(true).newXsltCompiler(); - - compiler.compile(source).load().transform(); - compiler.compile(source).load30().transform(null, null); - compiler.compile(source).load30().applyTemplates((Source) null); - compiler.compile(source).load30().applyTemplates((Source) null, null); - compiler.compile(source).load30().applyTemplates((XdmValue) null); - compiler.compile(source).load30().applyTemplates((XdmValue) null, null); - compiler.compile(source).load30().callFunction(null, null); - compiler.compile(source).load30().callFunction(null, null, null); - compiler.compile(source).load30().callTemplate(null); - compiler.compile(source).load30().callTemplate(null, null); + + compiler.compile(source).load().transform(); // $hasXsltInjection + compiler.compile(source).load30().transform(null, null); // $hasXsltInjection + compiler.compile(source).load30().applyTemplates((Source) null); // $hasXsltInjection + compiler.compile(source).load30().applyTemplates((Source) null, null); // $hasXsltInjection + compiler.compile(source).load30().applyTemplates((XdmValue) null); // $hasXsltInjection + compiler.compile(source).load30().applyTemplates((XdmValue) null, null); // $hasXsltInjection + compiler.compile(source).load30().callFunction(null, null); // $hasXsltInjection + compiler.compile(source).load30().callFunction(null, null, null); // $hasXsltInjection + compiler.compile(source).load30().callTemplate(null); // $hasXsltInjection + compiler.compile(source).load30().callTemplate(null, null); // $hasXsltInjection } @RequestMapping @@ -104,24 +108,24 @@ public void testSaxonXsltPackage(@RequestParam String param, Socket socket) thro URI uri = new URI(param); StreamSource source = new StreamSource(socket.getInputStream()); XsltCompiler compiler = new Processor(true).newXsltCompiler(); - - compiler.loadExecutablePackage(uri).load().transform(); - compiler.compilePackage(source).link().load().transform(); - compiler.loadLibraryPackage(uri).link().load().transform(); + + compiler.loadExecutablePackage(uri).load().transform(); // $hasXsltInjection + compiler.compilePackage(source).link().load().transform(); // $hasXsltInjection + compiler.loadLibraryPackage(uri).link().load().transform(); // $hasXsltInjection } public void testOkFeatureSecureProcessing(Socket socket) throws Exception { StreamSource source = new StreamSource(socket.getInputStream()); TransformerFactory factory = TransformerFactory.newInstance(); factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true); - factory.newTransformer(source).transform(null, null); + factory.newTransformer(source).transform(null, null); // Safe } public void testOkSaxon(Socket socket) throws Exception { StreamSource source = new StreamSource(socket.getInputStream()); XsltCompiler compiler = new Processor(true).newXsltCompiler(); - - compiler.compile(source).load().close(); - compiler.compile((Source) new Object()).load().transform(); + + compiler.compile(source).load().close(); // Safe + compiler.compile((Source) new Object()).load().transform(); // Safe } -} \ No newline at end of file +} diff --git a/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.ql b/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.ql new file mode 100644 index 000000000000..c2f02865b2e9 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-074/XsltInjectionTest.ql @@ -0,0 +1,22 @@ +import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.security.XsltInjectionQuery +import TestUtilities.InlineExpectationsTest + +class HasXsltInjectionTest extends InlineExpectationsTest { + HasXsltInjectionTest() { this = "HasXsltInjectionTest" } + + override string getARelevantTag() { result = "hasXsltInjection" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "hasXsltInjection" and + exists(DataFlow::Node src, DataFlow::Node sink, XsltInjectionFlowConfig conf | + conf.hasFlow(src, sink) + | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + } +} diff --git a/java/ql/test/query-tests/security/CWE-074/options b/java/ql/test/query-tests/security/CWE-074/options index efb24fabff15..a015486520ef 100644 --- a/java/ql/test/query-tests/security/CWE-074/options +++ b/java/ql/test/query-tests/security/CWE-074/options @@ -1 +1 @@ -//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/shiro-core-1.5.2:${testdir}/../../../stubs/spring-ldap-2.3.2 +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../stubs/springframework-5.3.8:${testdir}/../../../stubs/shiro-core-1.5.2:${testdir}/../../../stubs/spring-ldap-2.3.2:${testdir}/../../../stubs/Saxon-HE-9.9.1-7 diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/Configuration.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/Configuration.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/Configuration.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/Configuration.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/lib/SourceResolver.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/lib/SourceResolver.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/lib/SourceResolver.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/lib/SourceResolver.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/om/NotationSet.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/om/NotationSet.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/om/NotationSet.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/om/NotationSet.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/AbstractXsltTransformer.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/AbstractXsltTransformer.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/AbstractXsltTransformer.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/AbstractXsltTransformer.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Destination.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Destination.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Destination.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Destination.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Processor.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Processor.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Processor.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Processor.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/QName.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/QName.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/QName.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/QName.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiException.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiException.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiException.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiException.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiUncheckedException.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiUncheckedException.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiUncheckedException.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/SaxonApiUncheckedException.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmItem.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmItem.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmItem.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmItem.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmValue.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmValue.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmValue.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XdmValue.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Xslt30Transformer.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Xslt30Transformer.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Xslt30Transformer.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/Xslt30Transformer.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltCompiler.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltCompiler.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltCompiler.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltCompiler.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltExecutable.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltExecutable.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltExecutable.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltExecutable.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltPackage.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltPackage.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltPackage.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltPackage.java diff --git a/java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltTransformer.java b/java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltTransformer.java similarity index 100% rename from java/ql/test/experimental/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltTransformer.java rename to java/ql/test/stubs/Saxon-HE-9.9.1-7/net/sf/saxon/s9api/XsltTransformer.java