mirror of
https://github.com/github/codeql.git
synced 2026-02-09 19:51:07 +01:00
Add experimental variants of java/xxe, incorporating new sinks and a version that uses local sources.
Originally authored by @haby0, squashed to clean up a tangled commit history.
This commit is contained in:
Chris Smowton
85
java/ql/src/experimental/Security/CWE/CWE-611/XXE.java
Normal file
85
java/ql/src/experimental/Security/CWE/CWE-611/XXE.java
Normal file
@@ -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);
|
||||
}
|
||||
}
|
||||
67
java/ql/src/experimental/Security/CWE/CWE-611/XXE.qhelp
Normal file
67
java/ql/src/experimental/Security/CWE/CWE-611/XXE.qhelp
Normal file
@@ -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 & 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>
|
||||
30
java/ql/src/experimental/Security/CWE/CWE-611/XXE.ql
Normal file
30
java/ql/src/experimental/Security/CWE/CWE-611/XXE.ql
Normal file
@@ -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"
|
||||
262
java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll
Normal file
262
java/ql/src/experimental/Security/CWE/CWE-611/XXELib.qll
Normal file
@@ -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 }
|
||||
}
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="XXE.qhelp" /></qhelp>
|
||||
32
java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.ql
Normal file
32
java/ql/src/experimental/Security/CWE/CWE-611/XXELocal.ql
Normal file
@@ -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"
|
||||
Reference in New Issue
Block a user