Skip to content

Commit dc96d55

Browse files
authored
Merge pull request github#8888 from geoffw0/xxe2
C++: Add support for createLSParser to the CWE-611 XXE query.
2 parents 00b74d8 + d04078f commit dc96d55

File tree

3 files changed

+31
-10
lines changed

3 files changed

+31
-10
lines changed

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

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class XercesDOMParserFlowState extends XXEFlowState {
9191
}
9292

9393
/**
94-
* Flow state transformer for a call to
94+
* A flow state transformer for a call to
9595
* `AbstractDOMParser.setDisableDefaultEntityResolution`. Transforms the flow
9696
* state through the qualifier according to the setting in the parameter.
9797
*/
@@ -123,7 +123,7 @@ class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
123123
}
124124

125125
/**
126-
* Flow state transformer for a call to
126+
* A flow state transformer for a call to
127127
* `AbstractDOMParser.setCreateEntityReferenceNodes`. Transforms the flow
128128
* state through the qualifier according to the setting in the parameter.
129129
*/
@@ -162,7 +162,17 @@ class ParseFunction extends Function {
162162
}
163163

164164
/**
165-
* Configuration for tracking XML objects and their states.
165+
* The `createLSParser` function that returns a newly created `LSParser` object.
166+
*/
167+
class CreateLSParser extends Function {
168+
CreateLSParser() {
169+
this.hasName("createLSParser") and
170+
this.getUnspecifiedType().(PointerType).getBaseType().getName() = "DOMLSParser" // returns a `DOMLSParser *`.
171+
}
172+
}
173+
174+
/**
175+
* A configuration for tracking XML objects and their states.
166176
*/
167177
class XXEConfiguration extends DataFlow::Configuration {
168178
XXEConfiguration() { this = "XXEConfiguration" }
@@ -176,6 +186,13 @@ class XXEConfiguration extends DataFlow::Configuration {
176186
call.getThisArgument() and
177187
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
178188
)
189+
or
190+
// source is the result of a call to `createLSParser`.
191+
exists(Call call |
192+
call.getTarget() instanceof CreateLSParser and
193+
call = node.asExpr() and
194+
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
195+
)
179196
}
180197

181198
override predicate isSink(DataFlow::Node node, string flowstate) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ edges
2525
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:146:18:146:18 | q |
2626
| tests.cpp:144:18:144:18 | q | tests.cpp:130:39:130:39 | p |
2727
| tests.cpp:146:18:146:18 | q | tests.cpp:134:39:134:39 | p |
28+
| tests.cpp:150:19:150:32 | call to createLSParser | tests.cpp:152:2:152:2 | p |
2829
nodes
2930
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
3031
| tests.cpp:35:2:35:2 | p | semmle.label | p |
@@ -61,6 +62,8 @@ nodes
6162
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
6263
| tests.cpp:144:18:144:18 | q | semmle.label | q |
6364
| tests.cpp:146:18:146:18 | q | semmle.label | q |
65+
| tests.cpp:150:19:150:32 | call to createLSParser | semmle.label | call to createLSParser |
66+
| tests.cpp:152:2:152:2 | p | semmle.label | p |
6467
subpaths
6568
#select
6669
| 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 |
@@ -74,3 +77,4 @@ subpaths
7477
| 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 |
7578
| 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 |
7679
| 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 |
80+
| tests.cpp:152:2:152:2 | p | tests.cpp:150:19:150:32 | call to createLSParser | tests.cpp:152:2:152:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:150:19:150:32 | call to createLSParser | XML parser |

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ class XercesDOMParser: public AbstractDOMParser {
2222
XercesDOMParser();
2323
};
2424

25-
class LSParser: public AbstractDOMParser {
25+
class DOMLSParser : public AbstractDOMParser {
2626
};
2727

28-
LSParser *createLSParser();
28+
DOMLSParser *createLSParser();
2929

3030
// ---
3131

@@ -147,20 +147,20 @@ void test10(InputSource &data) {
147147
}
148148

149149
void test11(InputSource &data) {
150-
LSParser *p = createLSParser();
150+
DOMLSParser *p = createLSParser();
151151

152-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
152+
p->parse(data); // BAD (parser not correctly configured)
153153
}
154154

155155
void test12(InputSource &data) {
156-
LSParser *p = createLSParser();
156+
DOMLSParser *p = createLSParser();
157157

158158
p->setDisableDefaultEntityResolution(true);
159159
p->parse(data); // GOOD
160160
}
161161

162-
LSParser *g_p1 = createLSParser();
163-
LSParser *g_p2 = createLSParser();
162+
DOMLSParser *g_p1 = createLSParser();
163+
DOMLSParser *g_p2 = createLSParser();
164164
InputSource *g_data;
165165

166166
void test13() {

0 commit comments

Comments
 (0)