Skip to content

Commit b8fd07c

Browse files
MathiasVPMathiasVP
authored
Merge pull request #9018 from geoffw0/xxe5
C++: Support libxml2 in the XXE query
2 parents f65f833 + d5be11b commit b8fd07c

File tree

4 files changed

+207
-0
lines changed

4 files changed

+207
-0
lines changed

‎cpp/ql/src/Security/CWE/CWE-611/XXE.ql

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,42 @@ class CreateLSParser extends Function {
188188
}
189189
}
190190

191+
/**
192+
* A call to a `libxml2` function that parses XML.
193+
*/
194+
class Libxml2ParseCall extends FunctionCall {
195+
int optionsArg;
196+
197+
Libxml2ParseCall() {
198+
exists(string fname | this.getTarget().getName() = fname |
199+
fname = "xmlCtxtUseOptions" and optionsArg = 1
200+
or
201+
fname = "xmlReadFile" and optionsArg = 2
202+
or
203+
fname = ["xmlCtxtReadFile", "xmlParseInNodeContext", "xmlReadDoc", "xmlReadFd"] and
204+
optionsArg = 3
205+
or
206+
fname = ["xmlCtxtReadDoc", "xmlCtxtReadFd", "xmlReadMemory"] and optionsArg = 4
207+
or
208+
fname = ["xmlCtxtReadMemory", "xmlReadIO"] and optionsArg = 5
209+
or
210+
fname = "xmlCtxtReadIO" and optionsArg = 6
211+
)
212+
}
213+
214+
/**
215+
* Gets the argument that specifies `xmlParserOption`s.
216+
*/
217+
Expr getOptions() { result = this.getArgument(optionsArg) }
218+
}
219+
220+
/**
221+
* An `xmlParserOption` for `libxml2` that is considered unsafe.
222+
*/
223+
class Libxml2BadOption extends EnumConstant {
224+
Libxml2BadOption() { this.getName() = ["XML_PARSE_NOENT", "XML_PARSE_DTDLOAD"] }
225+
}
226+
191227
/**
192228
* A configuration for tracking XML objects and their states.
193229
*/
@@ -219,6 +255,23 @@ class XXEConfiguration extends DataFlow::Configuration {
219255
call.getThisArgument() and
220256
encodeXercesFlowState(flowstate, 0, 1) // default configuration
221257
)
258+
or
259+
// source is an `options` argument on a `libxml2` parse call that specifies
260+
// at least one unsafe option.
261+
//
262+
// note: we don't need to track an XML object for `libxml2`, so we don't
263+
// really need data flow. Nevertheless we jam it into this configuration,
264+
// with matching sources and sinks. This allows results to be presented by
265+
// the same query, in a consistent way as other results with flow paths.
266+
exists(Libxml2ParseCall call, Expr options |
267+
options = call.getOptions() and
268+
node.asExpr() = options and
269+
flowstate = "libxml2" and
270+
exists(Libxml2BadOption opt |
271+
globalValueNumber(options).getAnExpr().getValue().toInt().bitAnd(opt.getValue().toInt()) !=
272+
0
273+
)
274+
)
222275
}
223276

224277
override predicate isSink(DataFlow::Node node, string flowstate) {
@@ -229,6 +282,13 @@ class XXEConfiguration extends DataFlow::Configuration {
229282
) and
230283
flowstate instanceof XercesFlowState and
231284
not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
285+
or
286+
// sink is the `options` argument on a `libxml2` parse call.
287+
exists(Libxml2ParseCall call, Expr options |
288+
options = call.getOptions() and
289+
node.asExpr() = options and
290+
flowstate = "libxml2"
291+
)
232292
}
233293

234294
override predicate isAdditionalFlowStep(

‎cpp/ql/test/query-tests/Security/CWE/CWE-611/XXE.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ nodes
3333
| tests2.cpp:22:2:22:2 | p | semmle.label | p |
3434
| tests2.cpp:33:17:33:31 | SAXParser output argument | semmle.label | SAXParser output argument |
3535
| tests2.cpp:37:2:37:2 | p | semmle.label | p |
36+
| tests4.cpp:26:34:26:48 | (int)... | semmle.label | (int)... |
37+
| tests4.cpp:36:34:36:50 | (int)... | semmle.label | (int)... |
38+
| tests4.cpp:46:34:46:68 | ... \| ... | semmle.label | ... \| ... |
39+
| tests4.cpp:77:34:77:38 | flags | semmle.label | flags |
40+
| tests4.cpp:130:39:130:55 | (int)... | semmle.label | (int)... |
3641
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
3742
| tests.cpp:35:2:35:2 | p | semmle.label | p |
3843
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
@@ -74,6 +79,11 @@ subpaths
7479
#select
7580
| tests2.cpp:22:2:22:2 | p | tests2.cpp:20:17:20:31 | SAXParser output argument | tests2.cpp:22:2:22:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests2.cpp:20:17:20:31 | SAXParser output argument | XML parser |
7681
| tests2.cpp:37:2:37:2 | p | tests2.cpp:33:17:33:31 | SAXParser output argument | tests2.cpp:37:2:37:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests2.cpp:33:17:33:31 | SAXParser output argument | XML parser |
82+
| tests4.cpp:26:34:26:48 | (int)... | tests4.cpp:26:34:26:48 | (int)... | tests4.cpp:26:34:26:48 | (int)... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:26:34:26:48 | (int)... | XML parser |
83+
| tests4.cpp:36:34:36:50 | (int)... | tests4.cpp:36:34:36:50 | (int)... | tests4.cpp:36:34:36:50 | (int)... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:36:34:36:50 | (int)... | XML parser |
84+
| tests4.cpp:46:34:46:68 | ... \| ... | tests4.cpp:46:34:46:68 | ... \| ... | tests4.cpp:46:34:46:68 | ... \| ... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:46:34:46:68 | ... \| ... | XML parser |
85+
| tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | tests4.cpp:77:34:77:38 | flags | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:77:34:77:38 | flags | XML parser |
86+
| tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | tests4.cpp:130:39:130:55 | (int)... | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests4.cpp:130:39:130:55 | (int)... | XML parser |
7787
| 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 |
7888
| 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 |
7989
| 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 |

‎cpp/ql/test/query-tests/Security/CWE/CWE-611/tests.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@
22

33
class SecurityManager;
44
class InputSource;
5+
6+
#define NULL (0)

‎cpp/ql/test/query-tests/Security/CWE/CWE-611/tests4.cpp

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
// test cases for rule CWE-611 (libxml2)
2+
3+
#include "tests.h"
4+
5+
// ---
6+
7+
enum xmlParserOption
8+
{
9+
XML_PARSE_NOENT = 2,
10+
XML_PARSE_DTDLOAD = 4,
11+
XML_PARSE_OPTION_HARMLESS = 8
12+
};
13+
14+
class xmlDoc;
15+
16+
xmlDoc *xmlReadFile(const char *fileName, const char *encoding, int flags);
17+
xmlDoc *xmlReadMemory(const char *ptr, int sz, const char *url, const char *encoding, int flags);
18+
19+
void xmlFreeDoc(xmlDoc *ptr);
20+
21+
// ---
22+
23+
void test4_1(const char *fileName) {
24+
xmlDoc *p;
25+
26+
p = xmlReadFile(fileName, NULL, XML_PARSE_NOENT); // BAD (parser not correctly configured)
27+
if (p != NULL)
28+
{
29+
xmlFreeDoc(p);
30+
}
31+
}
32+
33+
void test4_2(const char *fileName) {
34+
xmlDoc *p;
35+
36+
p = xmlReadFile(fileName, NULL, XML_PARSE_DTDLOAD); // BAD (parser not correctly configured)
37+
if (p != NULL)
38+
{
39+
xmlFreeDoc(p);
40+
}
41+
}
42+
43+
void test4_3(const char *fileName) {
44+
xmlDoc *p;
45+
46+
p = xmlReadFile(fileName, NULL, XML_PARSE_NOENT | XML_PARSE_DTDLOAD); // BAD (parser not correctly configured)
47+
if (p != NULL)
48+
{
49+
xmlFreeDoc(p);
50+
}
51+
}
52+
53+
void test4_4(const char *fileName) {
54+
xmlDoc *p;
55+
56+
p = xmlReadFile(fileName, NULL, 0); // GOOD
57+
if (p != NULL)
58+
{
59+
xmlFreeDoc(p);
60+
}
61+
}
62+
63+
void test4_5(const char *fileName) {
64+
xmlDoc *p;
65+
66+
p = xmlReadFile(fileName, NULL, XML_PARSE_OPTION_HARMLESS); // GOOD
67+
if (p != NULL)
68+
{
69+
xmlFreeDoc(p);
70+
}
71+
}
72+
73+
void test4_6(const char *fileName) {
74+
xmlDoc *p;
75+
int flags = XML_PARSE_NOENT;
76+
77+
p = xmlReadFile(fileName, NULL, flags); // BAD (parser not correctly configured)
78+
if (p != NULL)
79+
{
80+
xmlFreeDoc(p);
81+
}
82+
}
83+
84+
void test4_7(const char *fileName) {
85+
xmlDoc *p;
86+
int flags = 0;
87+
88+
p = xmlReadFile(fileName, NULL, flags); // GOOD
89+
if (p != NULL)
90+
{
91+
xmlFreeDoc(p);
92+
}
93+
}
94+
95+
void test4_8(const char *fileName) {
96+
xmlDoc *p;
97+
int flags = XML_PARSE_OPTION_HARMLESS;
98+
99+
p = xmlReadFile(fileName, NULL, flags | XML_PARSE_NOENT); // BAD (parser not correctly configured) [NOT DETECTED]
100+
if (p != NULL)
101+
{
102+
xmlFreeDoc(p);
103+
}
104+
}
105+
106+
void test4_9(const char *fileName) {
107+
xmlDoc *p;
108+
int flags = XML_PARSE_NOENT;
109+
110+
p = xmlReadFile(fileName, NULL, flags | XML_PARSE_OPTION_HARMLESS); // BAD (parser not correctly configured) [NOT DETECTED]
111+
if (p != NULL)
112+
{
113+
xmlFreeDoc(p);
114+
}
115+
}
116+
117+
void test4_10(const char *ptr, int sz) {
118+
xmlDoc *p;
119+
120+
p = xmlReadMemory(ptr, sz, "", NULL, 0); // GOOD
121+
if (p != NULL)
122+
{
123+
xmlFreeDoc(p);
124+
}
125+
}
126+
127+
void test4_11(const char *ptr, int sz) {
128+
xmlDoc *p;
129+
130+
p = xmlReadMemory(ptr, sz, "", NULL, XML_PARSE_DTDLOAD); // BAD (parser not correctly configured)
131+
if (p != NULL)
132+
{
133+
xmlFreeDoc(p);
134+
}
135+
}

0 commit comments

Comments
 (0)