Skip to content

Commit 1e66a54

Browse files
atorralbaatorralba
committed
Promote exxperimental XXE sinks
1 parent 8b65937 commit 1e66a54

File tree

5 files changed

+204
-5
lines changed

5 files changed

+204
-5
lines changed

‎java/ql/lib/semmle/code/java/frameworks/apache/CommonsXml.qll

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/** Provides XML definitions related to the `org.apache.commons` package. */
2+
3+
import java
4+
private import semmle.code.java.dataflow.RangeUtils
5+
private import semmle.code.java.security.XmlParsers
6+
7+
/**
8+
* The classes `org.apache.commons.digester3.Digester`, `org.apache.commons.digester.Digester` or `org.apache.tomcat.util.digester.Digester`.
9+
*/
10+
private class Digester extends RefType {
11+
Digester() {
12+
this.hasQualifiedName([
13+
"org.apache.commons.digester3", "org.apache.commons.digester",
14+
"org.apache.tomcat.util.digester"
15+
], "Digester")
16+
}
17+
}
18+
19+
/** A call to `Digester.parse`. */
20+
private class DigesterParse extends XmlParserCall {
21+
DigesterParse() {
22+
exists(Method m |
23+
this.getMethod() = m and
24+
m.getDeclaringType() instanceof Digester and
25+
m.hasName("parse")
26+
)
27+
}
28+
29+
override Expr getSink() { result = this.getArgument(0) }
30+
31+
override predicate isSafe() { SafeDigesterFlow::flowToExpr(this.getQualifier()) }
32+
}
33+
34+
/** A `ParserConfig` that is specific to `Digester`. */
35+
private class DigesterConfig extends ParserConfig {
36+
DigesterConfig() {
37+
exists(Method m |
38+
m = this.getMethod() and
39+
m.getDeclaringType() instanceof Digester and
40+
m.hasName("setFeature")
41+
)
42+
}
43+
}
44+
45+
/**
46+
* A safely configured `Digester`.
47+
*/
48+
private class SafeDigester extends VarAccess {
49+
SafeDigester() {
50+
exists(Variable v | v = this.getVariable() |
51+
exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
52+
config.enables(singleSafeConfig())
53+
)
54+
or
55+
exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
56+
config
57+
.disables(any(ConstantStringExpr s |
58+
s.getStringValue() = "http://xml.org/sax/features/external-general-entities"
59+
))
60+
) and
61+
exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
62+
config
63+
.disables(any(ConstantStringExpr s |
64+
s.getStringValue() = "http://xml.org/sax/features/external-parameter-entities"
65+
))
66+
) and
67+
exists(DigesterConfig config | config.getQualifier() = v.getAnAccess() |
68+
config
69+
.disables(any(ConstantStringExpr s |
70+
s.getStringValue() =
71+
"http://apache.org/xml/features/nonvalidating/load-external-dtd"
72+
))
73+
)
74+
)
75+
}
76+
}
77+
78+
private module SafeDigesterFlowConfig implements DataFlow::ConfigSig {
79+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeDigester }
80+
81+
predicate isSink(DataFlow::Node sink) {
82+
exists(MethodAccess ma |
83+
sink.asExpr() = ma.getQualifier() and ma.getMethod().getDeclaringType() instanceof Digester
84+
)
85+
}
86+
87+
int fieldFlowBranchLimit() { result = 0 }
88+
}
89+
90+
private module SafeDigesterFlow = DataFlow::Global<SafeDigesterFlowConfig>;

‎java/ql/lib/semmle/code/java/frameworks/javaee/Xml.qll

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/** Provides definitions related to the `javax.xml` package. */
2+
3+
import java
4+
private import semmle.code.java.security.XmlParsers
5+
6+
/** A call to `Validator.validate`. */
7+
private class ValidatorValidate extends XmlParserCall {
8+
ValidatorValidate() {
9+
exists(Method m |
10+
this.getMethod() = m and
11+
m.getDeclaringType() instanceof Validator and
12+
m.hasName("validate")
13+
)
14+
}
15+
16+
override Expr getSink() { result = this.getArgument(0) }
17+
18+
override predicate isSafe() { SafeValidatorFlow::flowToExpr(this.getQualifier()) }
19+
}
20+
21+
/** A `TransformerConfig` specific to `Validator`. */
22+
private class ValidatorConfig extends TransformerConfig {
23+
ValidatorConfig() {
24+
exists(Method m |
25+
this.getMethod() = m and
26+
m.getDeclaringType() instanceof Validator and
27+
m.hasName("setProperty")
28+
)
29+
}
30+
}
31+
32+
/** The class `javax.xml.validation.Validator`. */
33+
private class Validator extends RefType {
34+
Validator() { this.hasQualifiedName("javax.xml.validation", "Validator") }
35+
}
36+
37+
/** A safely configured `Validator`. */
38+
private class SafeValidator extends VarAccess {
39+
SafeValidator() {
40+
exists(Variable v | v = this.getVariable() |
41+
exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() |
42+
config.disables(configAccessExternalDtd())
43+
) and
44+
exists(ValidatorConfig config | config.getQualifier() = v.getAnAccess() |
45+
config.disables(configAccessExternalSchema())
46+
)
47+
)
48+
}
49+
}
50+
51+
private module SafeValidatorFlowConfig implements DataFlow::ConfigSig {
52+
predicate isSource(DataFlow::Node src) { src.asExpr() instanceof SafeValidator }
53+
54+
predicate isSink(DataFlow::Node sink) {
55+
exists(MethodAccess ma |
56+
sink.asExpr() = ma.getQualifier() and
57+
ma.getMethod().getDeclaringType() instanceof Validator
58+
)
59+
}
60+
61+
int fieldFlowBranchLimit() { result = 0 }
62+
}
63+
64+
private module SafeValidatorFlow = DataFlow::Global<SafeValidatorFlowConfig>;

‎java/ql/lib/semmle/code/java/frameworks/javase/Beans.qll

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/** Provides definitions related to the `java.beans` package. */
2+
3+
import java
4+
private import semmle.code.java.security.XmlParsers
5+
6+
/** The class `java.beans.XMLDecoder`. */
7+
private class XmlDecoder extends RefType {
8+
XmlDecoder() { this.hasQualifiedName("java.beans", "XMLDecoder") }
9+
}
10+
11+
/** A call to `XMLDecoder.readObject`. */
12+
private class XmlDecoderReadObject extends XmlParserCall {
13+
XmlDecoderReadObject() {
14+
exists(Method m |
15+
this.getMethod() = m and
16+
m.getDeclaringType() instanceof XmlDecoder and
17+
m.hasName("readObject")
18+
)
19+
}
20+
21+
override Expr getSink() { result = this.getQualifier() }
22+
23+
override predicate isSafe() { none() }
24+
}

‎java/ql/lib/semmle/code/java/frameworks/rundeck/RundeckXml.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/** Provides definitions related to XML parsing in Rundeck. */
2+
3+
import java
4+
private import semmle.code.java.security.XmlParsers
5+
6+
/** A call to `ParserHelper.loadDocument`. */
7+
private class ParserHelperLoadDocument extends XmlParserCall {
8+
ParserHelperLoadDocument() {
9+
exists(Method m |
10+
this.getMethod() = m and
11+
m.getDeclaringType().hasQualifiedName("org.rundeck.api.parser", "ParserHelper") and
12+
m.hasName("loadDocument")
13+
)
14+
}
15+
16+
override Expr getSink() { result = this.getArgument(0) }
17+
18+
override predicate isSafe() { none() }
19+
}

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

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22

33
import java
44
import semmle.code.java.dataflow.DataFlow
5-
import semmle.code.java.dataflow.DataFlow2
65
import semmle.code.java.dataflow.DataFlow3
76
private import semmle.code.java.dataflow.RangeUtils
87

9-
/*
10-
* Various XML parsers in Java.
11-
*/
8+
private module Frameworks {
9+
private import semmle.code.java.frameworks.apache.CommonsXml
10+
private import semmle.code.java.frameworks.javaee.Xml
11+
private import semmle.code.java.frameworks.javase.Beans
12+
private import semmle.code.java.frameworks.rundeck.RundeckXml
13+
}
1214

1315
/**
1416
* An abstract type representing a call to parse XML files.
@@ -946,7 +948,7 @@ class TransformerFactorySource extends XmlParserCall {
946948
exists(Method m |
947949
this.getMethod() = m and
948950
m.getDeclaringType() instanceof TransformerFactory and
949-
m.hasName("newTransformer")
951+
m.hasName(["newTransformer", "newTransformerHandler"])
950952
)
951953
}
952954

0 commit comments

Comments
 (0)