diff --git a/java/ql/lib/semmle/code/java/security/XmlParsers.qll b/java/ql/lib/semmle/code/java/security/XmlParsers.qll index 5e2eb1caf8af..25ed99f52043 100644 --- a/java/ql/lib/semmle/code/java/security/XmlParsers.qll +++ b/java/ql/lib/semmle/code/java/security/XmlParsers.qll @@ -950,7 +950,11 @@ class TransformerFactoryConfig extends TransformerConfig { } } -private class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration { +/** + * A dataflow configuration that identifies `TransformerFactory` and `SAXTransformerFactory` + * instances that have been safely configured. + */ +class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration { SafeTransformerFactoryFlowConfig() { this = "XmlParsers::SafeTransformerFactoryFlowConfig" } override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformerFactory } diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXE.java b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.java new file mode 100644 index 000000000000..b56914235a72 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.java @@ -0,0 +1,85 @@ +import java.beans.XMLDecoder; +import java.io.BufferedReader; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; +import org.apache.commons.digester3.Digester; +import org.dom4j.Document; +import org.dom4j.DocumentHelper; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PostMapping; + +@Controller +public class XxeController { + + @PostMapping(value = "xxe1") + public void bad1(HttpServletRequest request, HttpServletResponse response) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + Digester digester = new Digester(); + digester.parse(servletInputStream); + } + + @PostMapping(value = "xxe2") + public void bad2(HttpServletRequest request) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str).append("\n"); + } + Document document = DocumentHelper.parseText(listString.toString()); + } + + @PostMapping(value = "xxe3") + public void bad3(HttpServletRequest request) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); + Schema schema = factory.newSchema(); + Validator validator = schema.newValidator(); + StreamSource source = new StreamSource(servletInputStream); + validator.validate(source); + } + + @PostMapping(value = "xxe4") + public void bad4(HttpServletRequest request) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + XMLDecoder xmlDecoder = new XMLDecoder(servletInputStream); + xmlDecoder.readObject(); + } + + @PostMapping(value = "good1") + public void good1(HttpServletRequest request, HttpServletResponse response) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str); + } + Digester digester = new Digester(); + digester.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + digester.setFeature("http://xml.org/sax/features/external-general-entities", false); + digester.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + digester.parse(listString.toString()); + } + + @PostMapping(value = "good2") + public void good2(HttpServletRequest request, HttpServletResponse response) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str).append("\n"); + } + SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); + Schema schema = factory.newSchema(); + Validator validator = schema.newValidator(); + validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalDTD", ""); + validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalSchema", ""); + StreamSource source = new StreamSource(listString.toString()); + validator.validate(source); + } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXE.qhelp b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.qhelp new file mode 100644 index 000000000000..c3cc04fdacb7 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.qhelp @@ -0,0 +1,67 @@ + + + + +

+Parsing untrusted XML files with a weakly configured XML parser may lead to an XML External Entity (XXE) attack. This type of attack +uses external entity references to access arbitrary files on a system, carry out denial of service, or server side +request forgery. Even when the result of parsing is not returned to the user, out-of-band +data retrieval techniques may allow attackers to steal sensitive data. Denial of services can also be +carried out in this situation. +

+

+There are many XML parsers for Java, and most of them are vulnerable to XXE because their default settings enable parsing of +external entities. This query currently identifies vulnerable XML parsing from the following parsers: javax.xml.validation.Validator, +org.dom4j.DocumentHelper, org.rundeck.api.parser.ParserHelper, org.apache.commons.digester3.Digester, +org.apache.commons.digester.Digester, org.apache.tomcat.util.digester.Digester, java.beans.XMLDecoder. +

+ + + +

+The best way to prevent XXE attacks is to disable the parsing of any Document Type Declarations (DTDs) in untrusted data. +If this is not possible you should disable the parsing of external general entities and external parameter entities. +This improves security but the code will still be at risk of denial of service and server side request forgery attacks. +Protection against denial of service attacks may also be implemented by setting entity expansion limits, which is done +by default in recent JDK and JRE implementations. +

+ + + +

+The following bad examples parses the xml data entered by the user under an unsafe configuration, which is inherently insecure and may cause xml entity injection. +In good examples, the security configuration is carried out, for example: Disable DTD to protect the program from XXE attacks. +

+ + + + + +
  • +OWASP vulnerability description: +XML External Entity (XXE) Processing. +
    • +
  • +OWASP guidance on parsing xml files: +XXE Prevention Cheat Sheet. +
    • +
  • +Paper by Timothy Morgen: +XML Schema, DTD, and Entity Attacks +
    • +
  • +Out-of-band data retrieval: Timur Yunusov & Alexey Osipov, Black hat EU 2013: +XML Out-Of-Band Data Retrieval. +
    • +
  • +Denial of service attack (Billion laughs): +Billion Laughs. +
    • +
  • +The Java Tutorials: +Processing Limit Definitions. +
    • + +     + +     diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXE.ql b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.ql new file mode 100644 index 000000000000..0e1fdd72223b --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXE.ql @@ -0,0 +1,30 @@ +/** + * @name Resolving XML external entity in user-controlled data (experimental sinks) + * @description Parsing user-controlled XML documents and allowing expansion of external entity + * references may lead to disclosure of confidential data or denial of service. + * (note this version differs from query `java/xxe` by including support for additional possibly-vulnerable XML parsers) + * @kind path-problem + * @problem.severity error + * @precision high + * @id java/xxe-with-experimental-sinks + * @tags security + * external/cwe/cwe-611 + */ + +import java +import XXELib +import semmle.code.java.dataflow.FlowSources +import DataFlow::PathGraph + +class XxeConfig extends TaintTracking::Configuration { + XxeConfig() { this = "XxeConfig" } + + override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(), + "user input" diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll b/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll new file mode 100644 index 000000000000..267b4bdef1d1 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll @@ -0,0 +1,262 @@ +import java +import semmle.code.java.dataflow.DataFlow3 +import semmle.code.java.dataflow.DataFlow4 +import semmle.code.java.dataflow.DataFlow5 +import semmle.code.java.security.XmlParsers +private import semmle.code.java.dataflow.SSA + +/** A data flow sink for untrusted user input used to insecure xml parse. */ +class UnsafeXxeSink extends DataFlow::ExprNode { + UnsafeXxeSink() { + exists(XmlParserCall parse | + parse.getSink() = this.getExpr() and + not parse.isSafe() + ) + } +} + +/** The class `org.rundeck.api.parser.ParserHelper`. */ +class ParserHelper extends RefType { + ParserHelper() { this.hasQualifiedName("org.rundeck.api.parser", "ParserHelper") } +} + +/** A call to `ParserHelper.loadDocument`. */ +class ParserHelperLoadDocument extends XmlParserCall { + ParserHelperLoadDocument() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof ParserHelper and + m.hasName("loadDocument") + ) + } + + override Expr getSink() { result = this.getArgument(0) } + + override predicate isSafe() { none() } +} + +/** The class `javax.xml.validation.Validator`. */ +class Validator extends RefType { + Validator() { this.hasQualifiedName("javax.xml.validation", "Validator") } +} + +/** A call to `Validator.validate`. */ +class ValidatorValidate extends XmlParserCall { + ValidatorValidate() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof Validator and + m.hasName("validate") + ) + } + + override Expr getSink() { result = this.getArgument(0) } + + override predicate isSafe() { + exists(SafeValidatorFlowConfig svfc | svfc.hasFlowToExpr(this.getQualifier())) + } +} + +/** A `ParserConfig` specific to `Validator`. */ +class ValidatorConfig extends TransformerConfig { + ValidatorConfig() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof Validator and + m.hasName("setProperty") + ) + } +} + +/** A safely configured `Validator`. */ +class SafeValidator extends VarAccess { + SafeValidator() { + exists(Variable v | v = this.getVariable() | + exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() | + config.disables(configAccessExternalDTD()) + ) and + exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() | + config.disables(configAccessExternalSchema()) + ) + ) + } +} + +private class SafeValidatorFlowConfig extends DataFlow3::Configuration { + SafeValidatorFlowConfig() { this = "SafeValidatorFlowConfig" } + + override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeValidator } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + sink.asExpr() = ma.getQualifier() and + ma.getMethod().getDeclaringType() instanceof Validator + ) + } + + override int fieldFlowBranchLimit() { result = 0 } +} + +/** The class `org.dom4j.DocumentHelper`. */ +class DocumentHelper extends RefType { + DocumentHelper() { this.hasQualifiedName("org.dom4j", "DocumentHelper") } +} + +/** A call to `DocumentHelper.parseText`. */ +class DocumentHelperParseText extends XmlParserCall { + DocumentHelperParseText() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof DocumentHelper and + m.hasName("parseText") + ) + } + + override Expr getSink() { result = this.getArgument(0) } + + override predicate isSafe() { none() } +} + +/** + * The classes `org.apache.commons.digester3.Digester`, `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`. + */ +class Digester extends RefType { + Digester() { + this.hasQualifiedName([ + "org.apache.commons.digester3", "org.apache.commons.digester", + "org.apache.tomcat.util.digester" + ], "Digester") + } +} + +/** A call to `Digester.parse`. */ +class DigesterParse extends XmlParserCall { + DigesterParse() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof Digester and + m.hasName("parse") + ) + } + + override Expr getSink() { result = this.getArgument(0) } + + override predicate isSafe() { + exists(SafeDigesterFlowConfig sdfc | sdfc.hasFlowToExpr(this.getQualifier())) + } +} + +/** A `ParserConfig` that is specific to `Digester`. */ +class DigesterConfig extends ParserConfig { + DigesterConfig() { + exists(Method m | + m = this.getMethod() and + m.getDeclaringType() instanceof Digester and + m.hasName("setFeature") + ) + } +} + +/** + * A safely configured `Digester`. + */ +class SafeDigester extends VarAccess { + SafeDigester() { + exists(Variable v | v = this.getVariable() | + exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() | + config.enables(singleSafeConfig()) + ) + or + exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() | + config + .disables(any(ConstantStringExpr s | + s.getStringValue() = "http://xml.org/sax/features/external-general-entities" + )) + ) and + exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() | + config + .disables(any(ConstantStringExpr s | + s.getStringValue() = "http://xml.org/sax/features/external-parameter-entities" + )) + ) and + exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() | + config + .disables(any(ConstantStringExpr s | + s.getStringValue() = + "http://apache.org/xml/features/nonvalidating/load-external-dtd" + )) + ) + ) + } +} + +private class SafeDigesterFlowConfig extends DataFlow4::Configuration { + SafeDigesterFlowConfig() { this = "SafeDigesterFlowConfig" } + + override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDigester } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma | + sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof Digester + ) + } + + override int fieldFlowBranchLimit() { result = 0 } +} + +/** The class `java.beans.XMLDecoder`. */ +class XMLDecoder extends RefType { + XMLDecoder() { this.hasQualifiedName("java.beans", "XMLDecoder") } +} + +/** A call to `XMLDecoder.readObject`. */ +class XMLDecoderReadObject extends XmlParserCall { + XMLDecoderReadObject() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType() instanceof XMLDecoder and + m.hasName("readObject") + ) + } + + override Expr getSink() { result = this.getQualifier() } + + override predicate isSafe() { none() } +} + +private predicate constantStringExpr(Expr e, string val) { + e.(CompileTimeConstantExpr).getStringValue() = val + or + exists(SsaExplicitUpdate v, Expr src | + e = v.getAUse() and + src = v.getDefiningExpr().(VariableAssign).getSource() and + constantStringExpr(src, val) + ) +} + +/** A call to `SAXTransformerFactory.newTransformerHandler`. */ +class SAXTransformerFactoryNewTransformerHandler extends XmlParserCall { + SAXTransformerFactoryNewTransformerHandler() { + exists(Method m | + this.getMethod() = m and + m.getDeclaringType().hasQualifiedName("javax.xml.transform.sax", "SAXTransformerFactory") and + m.hasName("newTransformerHandler") + ) + } + + override Expr getSink() { result = this.getArgument(0) } + + override predicate isSafe() { + exists(SafeTransformerFactoryFlowConfig stf | stf.hasFlowToExpr(this.getQualifier())) + } +} + +/** An expression that always has the same string value. */ +private class ConstantStringExpr extends Expr { + string value; + + ConstantStringExpr() { constantStringExpr(this, value) } + + /** Get the string value of this expression. */ + string getStringValue() { result = value } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.qhelp b/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.qhelp new file mode 100644 index 000000000000..4dc505dec6aa --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.qhelp @@ -0,0 +1,5 @@ + + + \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.ql b/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.ql new file mode 100644 index 000000000000..2d3ee9ec7856 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.ql @@ -0,0 +1,32 @@ +/** + * @name Resolving XML external entity from a local source (experimental sinks) + * @description Parsing user-controlled XML documents and allowing expansion of external entity + * references may lead to disclosure of confidential data or denial of service. + * (note this version differs from query `java/xxe` by including support for additional possibly-vulnerable XML parsers, + * and by considering local information sources dangerous (e.g. environment variables) in addition to the remote sources + * considered by the normal `java/xxe` query) + * @kind path-problem + * @problem.severity recommendation + * @precision medium + * @id java/xxe-local-experimental-sinks + * @tags security + * external/cwe/cwe-611 + */ + +import java +import XXELib +import semmle.code.java.dataflow.FlowSources +import DataFlow::PathGraph + +class XxeLocalConfig extends TaintTracking::Configuration { + XxeLocalConfig() { this = "XxeLocalConfig" } + + override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput } + + override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(), + "user input" diff --git a/java/ql/test/experimental/query-tests/security/CWE-611/XXE.expected b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.expected new file mode 100644 index 000000000000..0538cda4352f --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.expected @@ -0,0 +1,40 @@ +edges +| XXE.java:22:43:22:66 | getInputStream(...) : ServletInputStream | XXE.java:24:18:24:35 | servletInputStream | +| XXE.java:29:23:29:41 | getReader(...) : BufferedReader | XXE.java:32:17:32:18 | br : BufferedReader | +| XXE.java:32:17:32:18 | br : BufferedReader | XXE.java:32:17:32:29 | readLine(...) : String | +| XXE.java:32:17:32:29 | readLine(...) : String | XXE.java:33:22:33:24 | str : String | +| XXE.java:33:4:33:13 | listString [post update] : StringBuilder | XXE.java:35:48:35:57 | listString : StringBuilder | +| XXE.java:33:22:33:24 | str : String | XXE.java:33:4:33:13 | listString [post update] : StringBuilder | +| XXE.java:35:48:35:57 | listString : StringBuilder | XXE.java:35:48:35:68 | toString(...) | +| XXE.java:40:43:40:66 | getInputStream(...) : ServletInputStream | XXE.java:44:42:44:59 | servletInputStream : ServletInputStream | +| XXE.java:44:25:44:60 | new StreamSource(...) : StreamSource | XXE.java:45:22:45:27 | source | +| XXE.java:44:42:44:59 | servletInputStream : ServletInputStream | XXE.java:44:25:44:60 | new StreamSource(...) : StreamSource | +| XXE.java:50:43:50:66 | getInputStream(...) : ServletInputStream | XXE.java:51:42:51:59 | servletInputStream : ServletInputStream | +| XXE.java:51:27:51:60 | new XMLDecoder(...) : XMLDecoder | XXE.java:52:3:52:12 | xmlDecoder | +| XXE.java:51:42:51:59 | servletInputStream : ServletInputStream | XXE.java:51:27:51:60 | new XMLDecoder(...) : XMLDecoder | +nodes +| XXE.java:22:43:22:66 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XXE.java:24:18:24:35 | servletInputStream | semmle.label | servletInputStream | +| XXE.java:29:23:29:41 | getReader(...) : BufferedReader | semmle.label | getReader(...) : BufferedReader | +| XXE.java:32:17:32:18 | br : BufferedReader | semmle.label | br : BufferedReader | +| XXE.java:32:17:32:29 | readLine(...) : String | semmle.label | readLine(...) : String | +| XXE.java:33:4:33:13 | listString [post update] : StringBuilder | semmle.label | listString [post update] : StringBuilder | +| XXE.java:33:22:33:24 | str : String | semmle.label | str : String | +| XXE.java:35:48:35:57 | listString : StringBuilder | semmle.label | listString : StringBuilder | +| XXE.java:35:48:35:68 | toString(...) | semmle.label | toString(...) | +| XXE.java:40:43:40:66 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XXE.java:44:25:44:60 | new StreamSource(...) : StreamSource | semmle.label | new StreamSource(...) : StreamSource | +| XXE.java:44:42:44:59 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream | +| XXE.java:45:22:45:27 | source | semmle.label | source | +| XXE.java:50:43:50:66 | getInputStream(...) : ServletInputStream | semmle.label | getInputStream(...) : ServletInputStream | +| XXE.java:51:27:51:60 | new XMLDecoder(...) : XMLDecoder | semmle.label | new XMLDecoder(...) : XMLDecoder | +| XXE.java:51:42:51:59 | servletInputStream : ServletInputStream | semmle.label | servletInputStream : ServletInputStream | +| XXE.java:52:3:52:12 | xmlDecoder | semmle.label | xmlDecoder | +| XXE.java:57:49:57:72 | getInputStream(...) | semmle.label | getInputStream(...) | +subpaths +#select +| XXE.java:24:18:24:35 | servletInputStream | XXE.java:22:43:22:66 | getInputStream(...) : ServletInputStream | XXE.java:24:18:24:35 | servletInputStream | Unsafe parsing of XML file from $@. | XXE.java:22:43:22:66 | getInputStream(...) | user input | +| XXE.java:35:48:35:68 | toString(...) | XXE.java:29:23:29:41 | getReader(...) : BufferedReader | XXE.java:35:48:35:68 | toString(...) | Unsafe parsing of XML file from $@. | XXE.java:29:23:29:41 | getReader(...) | user input | +| XXE.java:45:22:45:27 | source | XXE.java:40:43:40:66 | getInputStream(...) : ServletInputStream | XXE.java:45:22:45:27 | source | Unsafe parsing of XML file from $@. | XXE.java:40:43:40:66 | getInputStream(...) | user input | +| XXE.java:52:3:52:12 | xmlDecoder | XXE.java:50:43:50:66 | getInputStream(...) : ServletInputStream | XXE.java:52:3:52:12 | xmlDecoder | Unsafe parsing of XML file from $@. | XXE.java:50:43:50:66 | getInputStream(...) | user input | +| XXE.java:57:49:57:72 | getInputStream(...) | XXE.java:57:49:57:72 | getInputStream(...) | XXE.java:57:49:57:72 | getInputStream(...) | Unsafe parsing of XML file from $@. | XXE.java:57:49:57:72 | getInputStream(...) | user input | diff --git a/java/ql/test/experimental/query-tests/security/CWE-611/XXE.java b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.java new file mode 100644 index 000000000000..b835b1c84e6f --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.java @@ -0,0 +1,91 @@ +import java.beans.XMLDecoder; +import java.io.BufferedReader; +import javax.servlet.ServletInputStream; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.Schema; +import javax.xml.validation.SchemaFactory; +import javax.xml.validation.Validator; +import org.rundeck.api.parser.ParserHelper; +import org.apache.commons.digester3.Digester; +import org.dom4j.Document; +import org.dom4j.DocumentHelper; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.PostMapping; + +@Controller +public class XXE { + + @PostMapping(value = "bad1") + public void bad1(HttpServletRequest request, HttpServletResponse response) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + Digester digester = new Digester(); + digester.parse(servletInputStream); //bad + } + + @PostMapping(value = "bad2") + public void bad2(HttpServletRequest request) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str).append("\n"); + } + Document document = DocumentHelper.parseText(listString.toString()); //bad + } + + @PostMapping(value = "bad3") + public void bad3(HttpServletRequest request) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); + Schema schema = factory.newSchema(); + Validator validator = schema.newValidator(); + StreamSource source = new StreamSource(servletInputStream); + validator.validate(source); //bad + } + + @PostMapping(value = "bad4") + public void bad4(HttpServletRequest request) throws Exception { + ServletInputStream servletInputStream = request.getInputStream(); + XMLDecoder xmlDecoder = new XMLDecoder(servletInputStream); + xmlDecoder.readObject(); //bad + } + + @PostMapping(value = "bad5") + public void bad5(HttpServletRequest request) throws Exception { + Document document = ParserHelper.loadDocument(request.getInputStream()); //bad + } + + @PostMapping(value = "good1") + public void good1(HttpServletRequest request, HttpServletResponse response) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str); + } + Digester digester = new Digester(); + digester.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + digester.setFeature("http://xml.org/sax/features/external-general-entities", false); + digester.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + digester.parse(listString.toString()); + } + + @PostMapping(value = "good2") + public void good2(HttpServletRequest request, HttpServletResponse response) throws Exception { + BufferedReader br = request.getReader(); + String str = ""; + StringBuilder listString = new StringBuilder(); + while ((str = br.readLine()) != null) { + listString.append(str).append("\n"); + } + SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema"); + Schema schema = factory.newSchema(); + Validator validator = schema.newValidator(); + validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalDTD", ""); + validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalSchema", ""); + StreamSource source = new StreamSource(listString.toString()); + validator.validate(source); + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-611/XXE.qlref b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.qlref new file mode 100644 index 000000000000..0675e245daa1 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-611/XXE.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-611/XXE.ql diff --git a/java/ql/test/experimental/query-tests/security/CWE-611/options b/java/ql/test/experimental/query-tests/security/CWE-611/options new file mode 100644 index 000000000000..9aea8cdbe505 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-611/options @@ -0,0 +1 @@ +//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/servlet-api-2.4/:${testdir}/../../../../stubs/springframework-5.3.8/:${testdir}/../../../../stubs/dom4j-2.1.1:${testdir}/../../../../stubs/apache-commons-digester3-3.2:${testdir}/../../../../stubs/jaxen-1.2.0/:${testdir}/../../../../stubs/rundeck-api-java-client-13.2 \ No newline at end of file diff --git a/java/ql/test/stubs/apache-commons-digester3-3.2/org/apache/commons/digester3/Digester.java b/java/ql/test/stubs/apache-commons-digester3-3.2/org/apache/commons/digester3/Digester.java new file mode 100644 index 000000000000..546aecbe521c --- /dev/null +++ b/java/ql/test/stubs/apache-commons-digester3-3.2/org/apache/commons/digester3/Digester.java @@ -0,0 +1,50 @@ +package org.apache.commons.digester3; + +import java.io.File; +import org.xml.sax.InputSource; +import java.io.Reader; +import java.net.URL; +import java.io.InputStream; +import java.io.IOException; +import org.xml.sax.SAXException; +import javax.xml.parsers.SAXParser; +import org.xml.sax.XMLReader; +import org.xml.sax.helpers.DefaultHandler; +import javax.xml.parsers.ParserConfigurationException; +import org.xml.sax.SAXNotRecognizedException; +import org.xml.sax.SAXNotSupportedException; + +public class Digester extends DefaultHandler { + + public Digester() { } + + public Digester(SAXParser parser) { } + + public Digester(XMLReader reader) { } + + public T parse(InputStream input) throws IOException, SAXException { + return null; + } + + public T parse(File file) throws IOException, SAXException { + return null; + } + + public T parse(InputSource input) throws IOException, SAXException { + return null; + } + + public T parse(Reader reader) throws IOException, SAXException { + return null; + } + + public T parse(String uri) throws IOException, SAXException { + return null; + } + + public T parse(URL url) throws IOException, SAXException { + return null; + } + + public void setFeature(String feature, boolean value) throws ParserConfigurationException, SAXNotRecognizedException, SAXNotSupportedException { } +} \ No newline at end of file diff --git a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentException.java b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentException.java new file mode 100644 index 000000000000..2afcfe6e3cda --- /dev/null +++ b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentException.java @@ -0,0 +1,24 @@ +package org.dom4j; + +public class DocumentException extends Exception { + public DocumentException() { + } + + public DocumentException(String message) { + super(message); + } + + public DocumentException(String message, Throwable cause) { + super(message, cause); + } + + public DocumentException(Throwable cause) { + super(cause); + } + + /** @deprecated */ + @Deprecated + public Throwable getNestedException() { + return null; + } +} diff --git a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentHelper.java b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentHelper.java index 669dbfa9e9bf..65254265774f 100644 --- a/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentHelper.java +++ b/java/ql/test/stubs/dom4j-2.1.1/org/dom4j/DocumentHelper.java @@ -69,6 +69,9 @@ public static Element makeElement(Branch source, String path) { return null; } + public static Document parseText(String text) throws DocumentException { + return null; + } } /* diff --git a/java/ql/test/stubs/rundeck-api-java-client-13.2/org/rundeck/api/parser/ParserHelper.java b/java/ql/test/stubs/rundeck-api-java-client-13.2/org/rundeck/api/parser/ParserHelper.java new file mode 100644 index 000000000000..34acd64f63e9 --- /dev/null +++ b/java/ql/test/stubs/rundeck-api-java-client-13.2/org/rundeck/api/parser/ParserHelper.java @@ -0,0 +1,13 @@ +package org.rundeck.api.parser; + +import java.io.InputStream; +import org.dom4j.Document; + +public class ParserHelper { + public ParserHelper() { + } + + public static Document loadDocument(InputStream inputStream) { + return null; + } +}