Skip to content

Commit 0ebbb33

Browse files
authored
Merge pull request github#6564 from haby0/java/xxe/new
Java: Add XXE sinks
2 parents db78e3a + 29028c5 commit 0ebbb33

File tree

15 files changed

+709
-1
lines changed

15 files changed

+709
-1
lines changed

java/ql/lib/semmle/code/java/security/XmlParsers.qll

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,11 @@ class TransformerFactoryConfig extends TransformerConfig {
950950
}
951951
}
952952

953-
private class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration {
953+
/**
954+
* A dataflow configuration that identifies `TransformerFactory` and `SAXTransformerFactory`
955+
* instances that have been safely configured.
956+
*/
957+
class SafeTransformerFactoryFlowConfig extends DataFlow3::Configuration {
954958
SafeTransformerFactoryFlowConfig() { this = "XmlParsers::SafeTransformerFactoryFlowConfig" }
955959

956960
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeTransformerFactory }
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
import java.beans.XMLDecoder;
2+
import java.io.BufferedReader;
3+
import javax.servlet.ServletInputStream;
4+
import javax.servlet.http.HttpServletRequest;
5+
import javax.servlet.http.HttpServletResponse;
6+
import javax.xml.transform.stream.StreamSource;
7+
import javax.xml.validation.Schema;
8+
import javax.xml.validation.SchemaFactory;
9+
import javax.xml.validation.Validator;
10+
import org.apache.commons.digester3.Digester;
11+
import org.dom4j.Document;
12+
import org.dom4j.DocumentHelper;
13+
import org.springframework.stereotype.Controller;
14+
import org.springframework.web.bind.annotation.PostMapping;
15+
16+
@Controller
17+
public class XxeController {
18+
19+
@PostMapping(value = "xxe1")
20+
public void bad1(HttpServletRequest request, HttpServletResponse response) throws Exception {
21+
ServletInputStream servletInputStream = request.getInputStream();
22+
Digester digester = new Digester();
23+
digester.parse(servletInputStream);
24+
}
25+
26+
@PostMapping(value = "xxe2")
27+
public void bad2(HttpServletRequest request) throws Exception {
28+
BufferedReader br = request.getReader();
29+
String str = "";
30+
StringBuilder listString = new StringBuilder();
31+
while ((str = br.readLine()) != null) {
32+
listString.append(str).append("\n");
33+
}
34+
Document document = DocumentHelper.parseText(listString.toString());
35+
}
36+
37+
@PostMapping(value = "xxe3")
38+
public void bad3(HttpServletRequest request) throws Exception {
39+
ServletInputStream servletInputStream = request.getInputStream();
40+
SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema");
41+
Schema schema = factory.newSchema();
42+
Validator validator = schema.newValidator();
43+
StreamSource source = new StreamSource(servletInputStream);
44+
validator.validate(source);
45+
}
46+
47+
@PostMapping(value = "xxe4")
48+
public void bad4(HttpServletRequest request) throws Exception {
49+
ServletInputStream servletInputStream = request.getInputStream();
50+
XMLDecoder xmlDecoder = new XMLDecoder(servletInputStream);
51+
xmlDecoder.readObject();
52+
}
53+
54+
@PostMapping(value = "good1")
55+
public void good1(HttpServletRequest request, HttpServletResponse response) throws Exception {
56+
BufferedReader br = request.getReader();
57+
String str = "";
58+
StringBuilder listString = new StringBuilder();
59+
while ((str = br.readLine()) != null) {
60+
listString.append(str);
61+
}
62+
Digester digester = new Digester();
63+
digester.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
64+
digester.setFeature("http://xml.org/sax/features/external-general-entities", false);
65+
digester.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
66+
digester.parse(listString.toString());
67+
}
68+
69+
@PostMapping(value = "good2")
70+
public void good2(HttpServletRequest request, HttpServletResponse response) throws Exception {
71+
BufferedReader br = request.getReader();
72+
String str = "";
73+
StringBuilder listString = new StringBuilder();
74+
while ((str = br.readLine()) != null) {
75+
listString.append(str).append("\n");
76+
}
77+
SchemaFactory factory = SchemaFactory.newInstance("http://www.w3.org/2001/XMLSchema");
78+
Schema schema = factory.newSchema();
79+
Validator validator = schema.newValidator();
80+
validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalDTD", "");
81+
validator.setProperty("http://javax.xml.XMLConstants/property/accessExternalSchema", "");
82+
StreamSource source = new StreamSource(listString.toString());
83+
validator.validate(source);
84+
}
85+
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Parsing untrusted XML files with a weakly configured XML parser may lead to an XML External Entity (XXE) attack. This type of attack
7+
uses external entity references to access arbitrary files on a system, carry out denial of service, or server side
8+
request forgery. Even when the result of parsing is not returned to the user, out-of-band
9+
data retrieval techniques may allow attackers to steal sensitive data. Denial of services can also be
10+
carried out in this situation.
11+
</p>
12+
<p>
13+
There are many XML parsers for Java, and most of them are vulnerable to XXE because their default settings enable parsing of
14+
external entities. This query currently identifies vulnerable XML parsing from the following parsers: <code>javax.xml.validation.Validator</code>,
15+
<code>org.dom4j.DocumentHelper</code>, <code>org.rundeck.api.parser.ParserHelper</code>, <code>org.apache.commons.digester3.Digester</code>,
16+
<code>org.apache.commons.digester.Digester</code>, <code>org.apache.tomcat.util.digester.Digester</code>, <code>java.beans.XMLDecoder</code>.
17+
</p>
18+
</overview>
19+
20+
<recommendation>
21+
<p>
22+
The best way to prevent XXE attacks is to disable the parsing of any Document Type Declarations (DTDs) in untrusted data.
23+
If this is not possible you should disable the parsing of external general entities and external parameter entities.
24+
This improves security but the code will still be at risk of denial of service and server side request forgery attacks.
25+
Protection against denial of service attacks may also be implemented by setting entity expansion limits, which is done
26+
by default in recent JDK and JRE implementations.
27+
</p>
28+
</recommendation>
29+
30+
<example>
31+
<p>
32+
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.
33+
In good examples, the security configuration is carried out, for example: Disable DTD to protect the program from XXE attacks.
34+
</p>
35+
<sample src="XXE.java" />
36+
</example>
37+
38+
<references>
39+
40+
<li>
41+
OWASP vulnerability description:
42+
<a href="https://www.owasp.org/index.php/XML_External_Entity_(XXE)_Processing">XML External Entity (XXE) Processing</a>.
43+
</li>
44+
<li>
45+
OWASP guidance on parsing xml files:
46+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html#java">XXE Prevention Cheat Sheet</a>.
47+
</li>
48+
<li>
49+
Paper by Timothy Morgen:
50+
<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>
51+
</li>
52+
<li>
53+
Out-of-band data retrieval: Timur Yunusov &amp; Alexey Osipov, Black hat EU 2013:
54+
<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
55+
</li>
56+
<li>
57+
Denial of service attack (Billion laughs):
58+
<a href="https://en.wikipedia.org/wiki/Billion_laughs">Billion Laughs.</a>
59+
</li>
60+
<li>
61+
The Java Tutorials:
62+
<a href="https://docs.oracle.com/javase/tutorial/jaxp/limits/limits.html">Processing Limit Definitions.</a>
63+
</li>
64+
65+
</references>
66+
67+
</qhelp>
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* @name Resolving XML external entity in user-controlled data (experimental sinks)
3+
* @description Parsing user-controlled XML documents and allowing expansion of external entity
4+
* references may lead to disclosure of confidential data or denial of service.
5+
* (note this version differs from query `java/xxe` by including support for additional possibly-vulnerable XML parsers)
6+
* @kind path-problem
7+
* @problem.severity error
8+
* @precision high
9+
* @id java/xxe-with-experimental-sinks
10+
* @tags security
11+
* external/cwe/cwe-611
12+
*/
13+
14+
import java
15+
import XXELib
16+
import semmle.code.java.dataflow.FlowSources
17+
import DataFlow::PathGraph
18+
19+
class XxeConfig extends TaintTracking::Configuration {
20+
XxeConfig() { this = "XxeConfig" }
21+
22+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
25+
}
26+
27+
from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
28+
where conf.hasFlowPath(source, sink)
29+
select sink.getNode(), source, sink, "Unsafe parsing of XML file from $@.", source.getNode(),
30+
"user input"

0 commit comments

Comments
 (0)