github · smowton · Sep 10, 2021 · Sep 10, 2021 · Sep 10, 2021 · smowton
@@ -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 }

@@ -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);
+	}
+}
@@ -0,0 +1,67 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>
+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.
+</p>
+<p>
+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: <code>javax.xml.validation.Validator</code>,
+<code>org.dom4j.DocumentHelper</code>, <code>org.rundeck.api.parser.ParserHelper</code>, <code>org.apache.commons.digester3.Digester</code>, 
+<code>org.apache.commons.digester.Digester</code>, <code>org.apache.tomcat.util.digester.Digester</code>, <code>java.beans.XMLDecoder</code>.
+</p>
+</overview>
+
+<recommendation>
+<p>
+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.
+</p>
+</recommendation>
+
+<example>
+<p>
+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.
+</p>
+<sample src="XXE.java" />
+</example>
+
+<references>
+
+<li>
+OWASP vulnerability description:
+<a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.
+</li>
+<li>
+OWASP guidance on parsing xml files:
+<a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java">XXE Prevention Cheat Sheet</a>.
+</li>
+<li>
+Paper by Timothy Morgen:
+<a href="https://research.nccgroup.com/2014/05/19/xml-schema-dtd-and-entity-attacks-a-compendium-of-known-techniques/">XML Schema, DTD, and Entity Attacks</a>
+</li>
+<li>
+Out-of-band data retrieval: Timur Yunusov &amp; Alexey Osipov, Black hat EU 2013:
+<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
+</li>
+<li>
+Denial of service attack (Billion laughs):
+<a href="https://en.wikipedia.org/wiki/Billion_laughs">Billion Laughs.</a>
+</li>
+<li>
+The Java Tutorials:
+<a href="https://docs.oracle.com/javase/tutorial/jaxp/limits/limits.html">Processing Limit Definitions.</a>
+</li>
+
+</references>
+
+</qhelp>
@@ -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.
- * @description Parsing user-controlled XML documents and allowing expansion of external entity
- *              references may lead to disclosure of confidential data or denial of service.
+ * @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)
- * @description Parsing user-controlled XML documents and allowing expansion of external entity
- *              references may lead to disclosure of confidential data or denial of service.
+ * @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)
+ *              (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"