Skip to content

Commit 1b06bf1

Browse files
atorralbaatorralba
authored
Merge pull request github#12932 from atorralba/atorralba/java/promote-xxe-experimental-sinks
Java: Promote experimental XXE sinks
2 parents cc36e3c + 770099f commit 1b06bf1

37 files changed

+584
-1194
lines changed

‎java/ql/lib/semmle/code/java/dataflow/RangeUtils.qll

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,17 @@ private predicate constantBooleanExpr(Expr e, boolean val) {
104104
CalcConstants::calculateBooleanValue(e) = val
105105
}
106106

107+
pragma[nomagic]
108+
private predicate constantStringExpr(Expr e, string val) {
109+
e.(CompileTimeConstantExpr).getStringValue() = val
110+
or
111+
exists(SsaExplicitUpdate v, Expr src |
112+
e = v.getAUse() and
113+
src = v.getDefiningExpr().(VariableAssign).getSource() and
114+
constantStringExpr(src, val)
115+
)
116+
}
117+
107118
private boolean getBoolValue(Expr e) { constantBooleanExpr(e, result) }
108119

109120
private int getIntValue(Expr e) { constantIntegerExpr(e, result) }
@@ -126,6 +137,14 @@ class ConstantBooleanExpr extends Expr {
126137
boolean getBooleanValue() { constantBooleanExpr(this, result) }
127138
}
128139

140+
/** An expression that always has the same string value. */
141+
class ConstantStringExpr extends Expr {
142+
ConstantStringExpr() { constantStringExpr(this, _) }
143+
144+
/** Get the string value of this expression. */
145+
string getStringValue() { constantStringExpr(this, result) }
146+
}
147+
129148
/**
130149
* Gets an expression that equals `v - d`.
131150
*/

‎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: 8 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +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
7-
import semmle.code.java.dataflow.DataFlow4
8-
import semmle.code.java.dataflow.DataFlow5
9-
private import semmle.code.java.dataflow.SSA
6+
private import semmle.code.java.dataflow.RangeUtils
107

11-
/*
12-
* Various XML parsers in Java.
13-
*/
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+
}
1414

1515
/**
1616
* An abstract type representing a call to parse XML files.
@@ -130,26 +130,6 @@ class DocumentBuilderFactoryConfig extends ParserConfig {
130130
}
131131
}
132132

133-
private predicate constantStringExpr(Expr e, string val) {
134-
e.(CompileTimeConstantExpr).getStringValue() = val
135-
or
136-
exists(SsaExplicitUpdate v, Expr src |
137-
e = v.getAUse() and
138-
src = v.getDefiningExpr().(VariableAssign).getSource() and
139-
constantStringExpr(src, val)
140-
)
141-
}
142-
143-
/** An expression that always has the same string value. */
144-
private class ConstantStringExpr extends Expr {
145-
string value;
146-
147-
ConstantStringExpr() { constantStringExpr(this, value) }
148-
149-
/** Get the string value of this expression. */
150-
string getStringValue() { result = value }
151-
}
152-
153133
/**
154134
* A general configuration that is safe when enabled.
155135
*/
@@ -973,7 +953,7 @@ class TransformerFactorySource extends XmlParserCall {
973953
exists(Method m |
974954
this.getMethod() = m and
975955
m.getDeclaringType() instanceof TransformerFactory and
976-
m.hasName("newTransformer")
956+
m.hasName(["newTransformer", "newTransformerHandler"])
977957
)
978958
}
979959

‎java/ql/src/change-notes/2023-04-26-xxe-sinks-promotion.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Experimental sinks for the query "Resolving XML external entity in user-controlled data" (`java/xxe`) have been promoted to the main query pack. These sinks were originally [submitted as part of an experimental query by @haby0](https://github.com/github/codeql/pull/6564).

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

Lines changed: 0 additions & 85 deletions
This file was deleted.

0 commit comments

Comments
 (0)