Skip to content

Commit 7ce040f

Browse files
authored
Merge pull request github#8736 from geoffw0/xxe
C++: New query for CWE-611 / XML External Entity Expansion (XXE)
2 parents 649d7dd + 7429491 commit 7ce040f

File tree

9 files changed

+528
-0
lines changed

9 files changed

+528
-0
lines changed
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
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
7+
XML external entity (XXE) attack. This type of attack uses external entity references
8+
to access arbitrary files on a system, carry out denial-of-service (DoS) attacks, or server-side
9+
request forgery. Even when the result of parsing is not returned to the user, DoS attacks are still possible
10+
and out-of-band data retrieval techniques may allow attackers to steal sensitive data.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
The easiest way to prevent XXE attacks is to disable external entity handling when
17+
parsing untrusted data. How this is done depends on the library being used. Note that some
18+
libraries, such as recent versions of <code>libxml</code>, disable entity expansion by default,
19+
so unless you have explicitly enabled entity expansion, no further action needs to be taken.
20+
</p>
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
The following example uses the <code>Xerces-C++</code> XML parser to parse a string <code>data</code>.
26+
If that string is from an untrusted source, this code may be vulnerable to an XXE attack, since
27+
the parser is constructed in its default state with <code>setDisableDefaultEntityResolution</code>
28+
set to <code>false</code>:
29+
</p>
30+
<sample src="XXEBad.cpp"/>
31+
32+
<p>
33+
To guard against XXE attacks, the <code>setDisableDefaultEntityResolution</code> option should be
34+
set to <code>true</code>.
35+
</p>
36+
<sample src="XXEGood.cpp"/>
37+
</example>
38+
39+
<references>
40+
<li>
41+
OWASP:
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:
46+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html">XML External Entity Prevention Cheat Sheet</a>.
47+
</li>
48+
<li>
49+
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+
Timur Yunusov, Alexey Osipov:
54+
<a href="https://www.slideshare.net/qqlan/bh-ready-v4">XML Out-Of-Band Data Retrieval</a>.
55+
</li>
56+
</references>
57+
</qhelp>
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/**
2+
* @name XML external entity expansion
3+
* @description Parsing user-controlled XML documents and allowing expansion of
4+
* external entity references may lead to disclosure of
5+
* confidential data or denial of service.
6+
* @kind path-problem
7+
* @id cpp/external-entity-expansion
8+
* @problem.severity warning
9+
* @security-severity 9.1
10+
* @precision medium
11+
* @tags security
12+
* external/cwe/cwe-611
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.ir.dataflow.DataFlow
17+
import DataFlow::PathGraph
18+
import semmle.code.cpp.ir.IR
19+
20+
/**
21+
* A flow state representing a possible configuration of an XML object.
22+
*/
23+
abstract class XXEFlowState extends DataFlow::FlowState {
24+
bindingset[this]
25+
XXEFlowState() { any() } // required characteristic predicate
26+
}
27+
28+
/**
29+
* An `Expr` that changes the configuration of an XML object, transforming the
30+
* `XXEFlowState` that flows through it.
31+
*/
32+
abstract class XXEFlowStateTranformer extends Expr {
33+
/**
34+
* Gets the flow state that `flowstate` is transformed into.
35+
*
36+
* Due to limitations of the implementation the transformation defined by this
37+
* predicate must be idempotent, that is, for any input `x` it must be that:
38+
* ```
39+
* transform(tranform(x)) = tranform(x)
40+
* ```
41+
*/
42+
abstract XXEFlowState transform(XXEFlowState flowstate);
43+
}
44+
45+
/**
46+
* The `AbstractDOMParser` class.
47+
*/
48+
class AbstractDOMParserClass extends Class {
49+
AbstractDOMParserClass() { this.hasName("AbstractDOMParser") }
50+
}
51+
52+
/**
53+
* The `XercesDOMParser` class.
54+
*/
55+
class XercesDOMParserClass extends Class {
56+
XercesDOMParserClass() { this.hasName("XercesDOMParser") }
57+
}
58+
59+
/**
60+
* Gets a valid flow state for `XercesDOMParser` flow.
61+
*
62+
* These flow states take the form `XercesDOM-A-B`, where:
63+
* - A is 1 if `setDisableDefaultEntityResolution` is `true`, 0 otherwise.
64+
* - B is 1 if `setCreateEntityReferenceNodes` is `true`, 0 otherwise.
65+
*/
66+
predicate encodeXercesDOMFlowState(
67+
string flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
68+
) {
69+
flowstate = "XercesDOM-0-0" and
70+
disabledDefaultEntityResolution = 0 and
71+
createEntityReferenceNodes = 0
72+
or
73+
flowstate = "XercesDOM-0-1" and
74+
disabledDefaultEntityResolution = 0 and
75+
createEntityReferenceNodes = 1
76+
or
77+
flowstate = "XercesDOM-1-0" and
78+
disabledDefaultEntityResolution = 1 and
79+
createEntityReferenceNodes = 0
80+
or
81+
flowstate = "XercesDOM-1-1" and
82+
disabledDefaultEntityResolution = 1 and
83+
createEntityReferenceNodes = 1
84+
}
85+
86+
/**
87+
* A flow state representing the configuration of a `XercesDOMParser` object.
88+
*/
89+
class XercesDOMParserFlowState extends XXEFlowState {
90+
XercesDOMParserFlowState() { encodeXercesDOMFlowState(this, _, _) }
91+
}
92+
93+
/**
94+
* Flow state transformer for a call to
95+
* `AbstractDOMParser.setDisableDefaultEntityResolution`. Transforms the flow
96+
* state through the qualifier according to the setting in the parameter.
97+
*/
98+
class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
99+
Expr newValue;
100+
101+
DisableDefaultEntityResolutionTranformer() {
102+
exists(Call call, Function f |
103+
call.getTarget() = f and
104+
f.getDeclaringType() instanceof AbstractDOMParserClass and
105+
f.hasName("setDisableDefaultEntityResolution") and
106+
this = call.getQualifier() and
107+
newValue = call.getArgument(0)
108+
)
109+
}
110+
111+
final override XXEFlowState transform(XXEFlowState flowstate) {
112+
exists(int createEntityReferenceNodes |
113+
encodeXercesDOMFlowState(flowstate, _, createEntityReferenceNodes) and
114+
(
115+
newValue.getValue().toInt() = 1 and // true
116+
encodeXercesDOMFlowState(result, 1, createEntityReferenceNodes)
117+
or
118+
not newValue.getValue().toInt() = 1 and // false or unknown
119+
encodeXercesDOMFlowState(result, 0, createEntityReferenceNodes)
120+
)
121+
)
122+
}
123+
}
124+
125+
/**
126+
* Flow state transformer for a call to
127+
* `AbstractDOMParser.setCreateEntityReferenceNodes`. Transforms the flow
128+
* state through the qualifier according to the setting in the parameter.
129+
*/
130+
class CreateEntityReferenceNodesTranformer extends XXEFlowStateTranformer {
131+
Expr newValue;
132+
133+
CreateEntityReferenceNodesTranformer() {
134+
exists(Call call, Function f |
135+
call.getTarget() = f and
136+
f.getDeclaringType() instanceof AbstractDOMParserClass and
137+
f.hasName("setCreateEntityReferenceNodes") and
138+
this = call.getQualifier() and
139+
newValue = call.getArgument(0)
140+
)
141+
}
142+
143+
final override XXEFlowState transform(XXEFlowState flowstate) {
144+
exists(int disabledDefaultEntityResolution |
145+
encodeXercesDOMFlowState(flowstate, disabledDefaultEntityResolution, _) and
146+
(
147+
newValue.getValue().toInt() = 1 and // true
148+
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 1)
149+
or
150+
not newValue.getValue().toInt() = 1 and // false or unknown
151+
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 0)
152+
)
153+
)
154+
}
155+
}
156+
157+
/**
158+
* The `AbstractDOMParser.parse` method.
159+
*/
160+
class ParseFunction extends Function {
161+
ParseFunction() { this.getClassAndName("parse") instanceof AbstractDOMParserClass }
162+
}
163+
164+
/**
165+
* Configuration for tracking XML objects and their states.
166+
*/
167+
class XXEConfiguration extends DataFlow::Configuration {
168+
XXEConfiguration() { this = "XXEConfiguration" }
169+
170+
override predicate isSource(DataFlow::Node node, string flowstate) {
171+
// source is the write on `this` of a call to the `XercesDOMParser`
172+
// constructor.
173+
exists(CallInstruction call |
174+
call.getStaticCallTarget() = any(XercesDOMParserClass c).getAConstructor() and
175+
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
176+
call.getThisArgument() and
177+
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
178+
)
179+
}
180+
181+
override predicate isSink(DataFlow::Node node, string flowstate) {
182+
// sink is the read of the qualifier of a call to `parse`.
183+
exists(Call call |
184+
call.getTarget() instanceof ParseFunction and
185+
call.getQualifier() = node.asConvertedExpr()
186+
) and
187+
flowstate instanceof XercesDOMParserFlowState and
188+
not encodeXercesDOMFlowState(flowstate, 1, 1) // safe configuration
189+
}
190+
191+
override predicate isAdditionalFlowStep(
192+
DataFlow::Node node1, string state1, DataFlow::Node node2, string state2
193+
) {
194+
// create additional flow steps for `XXEFlowStateTranformer`s
195+
state2 = node2.asConvertedExpr().(XXEFlowStateTranformer).transform(state1) and
196+
DataFlow::simpleLocalFlowStep(node1, node2)
197+
}
198+
199+
override predicate isBarrier(DataFlow::Node node, string flowstate) {
200+
// when the flowstate is transformed at a call node, block the original
201+
// flowstate value.
202+
node.asConvertedExpr().(XXEFlowStateTranformer).transform(flowstate) != flowstate
203+
}
204+
}
205+
206+
from XXEConfiguration conf, DataFlow::PathNode source, DataFlow::PathNode sink
207+
where conf.hasFlowPath(source, sink)
208+
select sink, source, sink,
209+
"This $@ is not configured to prevent an XML external entity (XXE) attack.", source, "XML parser"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
2+
XercesDOMParser *parser = new XercesDOMParser();
3+
4+
parser->parse(data); // BAD (parser is not correctly configured, may expand external entity references)
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
2+
XercesDOMParser *parser = new XercesDOMParser();
3+
4+
parser->setDisableDefaultEntityResolution(true);
5+
parser->parse(data);
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* An new query `cpp/external-entity-expansion` has been added. The query detects XML objects that are vulnerable to external entity expansion (XXE) attacks.
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
edges
2+
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p |
3+
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p |
4+
| tests.cpp:53:19:53:19 | VariableAddress [post update] | tests.cpp:55:2:55:2 | p |
5+
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | tests.cpp:53:19:53:19 | VariableAddress [post update] |
6+
| tests.cpp:55:2:55:2 | p | tests.cpp:56:2:56:2 | p |
7+
| tests.cpp:56:2:56:2 | p | tests.cpp:57:2:57:2 | p |
8+
| tests.cpp:69:19:69:19 | VariableAddress [post update] | tests.cpp:71:2:71:2 | p |
9+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:69:19:69:19 | VariableAddress [post update] |
10+
| tests.cpp:71:2:71:2 | p | tests.cpp:72:2:72:2 | p |
11+
| tests.cpp:72:2:72:2 | p | tests.cpp:73:2:73:2 | p |
12+
| tests.cpp:73:2:73:2 | p | tests.cpp:74:2:74:2 | p |
13+
| tests.cpp:73:2:73:2 | p | tests.cpp:74:2:74:2 | p |
14+
| tests.cpp:74:2:74:2 | p | tests.cpp:75:2:75:2 | p |
15+
| tests.cpp:75:2:75:2 | p | tests.cpp:76:2:76:2 | p |
16+
| tests.cpp:76:2:76:2 | p | tests.cpp:77:2:77:2 | p |
17+
| tests.cpp:77:2:77:2 | p | tests.cpp:78:2:78:2 | p |
18+
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p |
19+
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p |
20+
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q |
21+
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | tests.cpp:122:3:122:3 | q |
22+
| tests.cpp:130:39:130:39 | p | tests.cpp:131:2:131:2 | p |
23+
| tests.cpp:134:39:134:39 | p | tests.cpp:135:2:135:2 | p |
24+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:144:18:144:18 | q |
25+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:146:18:146:18 | q |
26+
| tests.cpp:144:18:144:18 | q | tests.cpp:130:39:130:39 | p |
27+
| tests.cpp:146:18:146:18 | q | tests.cpp:134:39:134:39 | p |
28+
nodes
29+
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
30+
| tests.cpp:35:2:35:2 | p | semmle.label | p |
31+
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
32+
| tests.cpp:49:2:49:2 | p | semmle.label | p |
33+
| tests.cpp:53:19:53:19 | VariableAddress [post update] | semmle.label | VariableAddress [post update] |
34+
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
35+
| tests.cpp:55:2:55:2 | p | semmle.label | p |
36+
| tests.cpp:56:2:56:2 | p | semmle.label | p |
37+
| tests.cpp:57:2:57:2 | p | semmle.label | p |
38+
| tests.cpp:69:19:69:19 | VariableAddress [post update] | semmle.label | VariableAddress [post update] |
39+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
40+
| tests.cpp:71:2:71:2 | p | semmle.label | p |
41+
| tests.cpp:72:2:72:2 | p | semmle.label | p |
42+
| tests.cpp:73:2:73:2 | p | semmle.label | p |
43+
| tests.cpp:74:2:74:2 | p | semmle.label | p |
44+
| tests.cpp:74:2:74:2 | p | semmle.label | p |
45+
| tests.cpp:75:2:75:2 | p | semmle.label | p |
46+
| tests.cpp:76:2:76:2 | p | semmle.label | p |
47+
| tests.cpp:77:2:77:2 | p | semmle.label | p |
48+
| tests.cpp:78:2:78:2 | p | semmle.label | p |
49+
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
50+
| tests.cpp:87:2:87:2 | p | semmle.label | p |
51+
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
52+
| tests.cpp:98:2:98:2 | p | semmle.label | p |
53+
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
54+
| tests.cpp:106:3:106:3 | q | semmle.label | q |
55+
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
56+
| tests.cpp:122:3:122:3 | q | semmle.label | q |
57+
| tests.cpp:130:39:130:39 | p | semmle.label | p |
58+
| tests.cpp:131:2:131:2 | p | semmle.label | p |
59+
| tests.cpp:134:39:134:39 | p | semmle.label | p |
60+
| tests.cpp:135:2:135:2 | p | semmle.label | p |
61+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
62+
| tests.cpp:144:18:144:18 | q | semmle.label | q |
63+
| tests.cpp:146:18:146:18 | q | semmle.label | q |
64+
subpaths
65+
#select
66+
| tests.cpp:35:2:35:2 | p | tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:33:23:33:43 | XercesDOMParser output argument | XML parser |
67+
| tests.cpp:49:2:49:2 | p | tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:46:23:46:43 | XercesDOMParser output argument | XML parser |
68+
| tests.cpp:57:2:57:2 | p | tests.cpp:53:23:53:43 | XercesDOMParser output argument | tests.cpp:57:2:57:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:53:23:53:43 | XercesDOMParser output argument | XML parser |
69+
| tests.cpp:74:2:74:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:74:2:74:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
70+
| tests.cpp:78:2:78:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:78:2:78:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
71+
| tests.cpp:87:2:87:2 | p | tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:84:23:84:43 | XercesDOMParser output argument | XML parser |
72+
| tests.cpp:98:2:98:2 | p | tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:91:23:91:43 | XercesDOMParser output argument | XML parser |
73+
| tests.cpp:106:3:106:3 | q | tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:103:24:103:44 | XercesDOMParser output argument | XML parser |
74+
| tests.cpp:122:3:122:3 | q | tests.cpp:118:24:118:44 | XercesDOMParser output argument | tests.cpp:122:3:122:3 | q | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:118:24:118:44 | XercesDOMParser output argument | XML parser |
75+
| tests.cpp:131:2:131:2 | p | tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:131:2:131:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |
76+
| tests.cpp:135:2:135:2 | p | tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:135:2:135:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-611/XXE.ql

0 commit comments

Comments
 (0)