Skip to content

Commit 5555b5c

Browse files
committed
Add local version of the XXE query
1 parent d55e9d5 commit 5555b5c

File tree

4 files changed

+85
-33
lines changed

4 files changed

+85
-33
lines changed
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/** Provides taint tracking configurations to be used in XXE queries. */
2+
3+
import java
4+
import semmle.code.java.dataflow.FlowSources
5+
import semmle.code.java.dataflow.TaintTracking
6+
import semmle.code.java.dataflow.TaintTracking2
7+
import semmle.code.java.security.XmlParsers
8+
import DataFlow::PathGraph
9+
10+
/**
11+
* A taint-tracking configuration for unvalidated remote user input that is used in XML external entity expansion.
12+
*/
13+
class XxeConfig extends TaintTracking::Configuration {
14+
XxeConfig() { this = "XxeConfig" }
15+
16+
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
17+
18+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
19+
}
20+
21+
/**
22+
* A taint-tracking configuration for unvalidated local user input that is used in XML external entity expansion.
23+
*/
24+
class XxeLocalConfig extends TaintTracking::Configuration {
25+
XxeLocalConfig() { this = "XxeLocalConfig" }
26+
27+
override predicate isSource(DataFlow::Node src) { src instanceof LocalUserInput }
28+
29+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
30+
}
31+
32+
private class UnsafeXxeSink extends DataFlow::ExprNode {
33+
UnsafeXxeSink() {
34+
not exists(SafeSaxSourceFlowConfig safeSource | safeSource.hasFlowTo(this)) and
35+
exists(XmlParserCall parse |
36+
parse.getSink() = this.getExpr() and
37+
not parse.isSafe()
38+
)
39+
}
40+
}
41+
42+
/**
43+
* A taint-tracking configuration for safe XML readers used to parse XML documents.
44+
*/
45+
private class SafeSaxSourceFlowConfig extends TaintTracking2::Configuration {
46+
SafeSaxSourceFlowConfig() { this = "XmlParsers::SafeSAXSourceFlowConfig" }
47+
48+
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource }
49+
50+
override predicate isSink(DataFlow::Node sink) {
51+
sink.asExpr() = any(XmlParserCall parse).getSink()
52+
}
53+
54+
override int fieldFlowBranchLimit() { result = 0 }
55+
}

java/ql/src/Security/CWE/CWE-611/XXE.ql

Lines changed: 1 addition & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -14,41 +14,9 @@
1414
*/
1515

1616
import java
17-
import semmle.code.java.security.XmlParsers
18-
import semmle.code.java.dataflow.FlowSources
19-
import semmle.code.java.dataflow.TaintTracking2
17+
import semmle.code.java.security.XxeQuery
2018
import DataFlow::PathGraph
2119

22-
class SafeSaxSourceFlowConfig extends TaintTracking2::Configuration {
23-
SafeSaxSourceFlowConfig() { this = "XmlParsers::SafeSAXSourceFlowConfig" }
24-
25-
override predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeSaxSource }
26-
27-
override predicate isSink(DataFlow::Node sink) {
28-
sink.asExpr() = any(XmlParserCall parse).getSink()
29-
}
30-
31-
override int fieldFlowBranchLimit() { result = 0 }
32-
}
33-
34-
class UnsafeXxeSink extends DataFlow::ExprNode {
35-
UnsafeXxeSink() {
36-
not exists(SafeSaxSourceFlowConfig safeSource | safeSource.hasFlowTo(this)) and
37-
exists(XmlParserCall parse |
38-
parse.getSink() = this.getExpr() and
39-
not parse.isSafe()
40-
)
41-
}
42-
}
43-
44-
class XxeConfig extends TaintTracking::Configuration {
45-
XxeConfig() { this = "XXE.ql::XxeConfig" }
46-
47-
override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
48-
49-
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeXxeSink }
50-
}
51-
5220
from DataFlow::PathNode source, DataFlow::PathNode sink, XxeConfig conf
5321
where conf.hasFlowPath(source, sink)
5422
select sink.getNode(), source, sink,
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="XXE.qhelp" /></qhelp>
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Resolving XML external entity in user-controlled data from local source
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+
* @kind path-problem
6+
* @problem.severity recommendation
7+
* @security-severity 9.1
8+
* @precision medium
9+
* @id java/xxe-local
10+
* @tags security
11+
* external/cwe/cwe-611
12+
* external/cwe/cwe-776
13+
* external/cwe/cwe-827
14+
*/
15+
16+
import java
17+
import semmle.code.java.security.XxeQuery
18+
import DataFlow::PathGraph
19+
20+
from DataFlow::PathNode source, DataFlow::PathNode sink, XxeLocalConfig conf
21+
where conf.hasFlowPath(source, sink)
22+
select sink.getNode(), source, sink,
23+
"XML parsing depends on a $@ without guarding against external entity expansion.",
24+
source.getNode(), "user-provided value"

0 commit comments

Comments
 (0)