Skip to content

Commit 2ccd5a5

Browse files
geoffw0geoffw0
committed
C++: Add support for SAXParser in the query.
1 parent 4e2344c commit 2ccd5a5

File tree

3 files changed

+65
-28
lines changed

3 files changed

+65
-28
lines changed

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

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -57,42 +57,51 @@ class XercesDOMParserClass extends Class {
5757
}
5858

5959
/**
60-
* Gets a valid flow state for `XercesDOMParser` flow.
60+
* The `SAXParser` class.
61+
*/
62+
class SAXParser extends Class {
63+
SAXParser() { this.hasName("SAXParser") }
64+
}
65+
66+
/**
67+
* Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
6168
*
62-
* These flow states take the form `XercesDOM-A-B`, where:
69+
* These flow states take the form `Xerces-A-B`, where:
6370
* - A is 1 if `setDisableDefaultEntityResolution` is `true`, 0 otherwise.
6471
* - B is 1 if `setCreateEntityReferenceNodes` is `true`, 0 otherwise.
6572
*/
66-
predicate encodeXercesDOMFlowState(
73+
predicate encodeXercesFlowState(
6774
string flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
6875
) {
69-
flowstate = "XercesDOM-0-0" and
76+
flowstate = "Xerces-0-0" and
7077
disabledDefaultEntityResolution = 0 and
7178
createEntityReferenceNodes = 0
7279
or
73-
flowstate = "XercesDOM-0-1" and
80+
flowstate = "Xerces-0-1" and
7481
disabledDefaultEntityResolution = 0 and
7582
createEntityReferenceNodes = 1
7683
or
77-
flowstate = "XercesDOM-1-0" and
84+
flowstate = "Xerces-1-0" and
7885
disabledDefaultEntityResolution = 1 and
7986
createEntityReferenceNodes = 0
8087
or
81-
flowstate = "XercesDOM-1-1" and
88+
flowstate = "Xerces-1-1" and
8289
disabledDefaultEntityResolution = 1 and
8390
createEntityReferenceNodes = 1
8491
}
8592

8693
/**
87-
* A flow state representing the configuration of a `XercesDOMParser` object.
94+
* A flow state representing the configuration of a `AbstractDOMParser` or
95+
* `SAXParser` object.
8896
*/
89-
class XercesDOMParserFlowState extends XXEFlowState {
90-
XercesDOMParserFlowState() { encodeXercesDOMFlowState(this, _, _) }
97+
class XercesFlowState extends XXEFlowState {
98+
XercesFlowState() { encodeXercesFlowState(this, _, _) }
9199
}
92100

93101
/**
94102
* A flow state transformer for a call to
95-
* `AbstractDOMParser.setDisableDefaultEntityResolution`. Transforms the flow
103+
* `AbstractDOMParser.setDisableDefaultEntityResolution` or
104+
* `SAXParser.setDisableDefaultEntityResolution`. Transforms the flow
96105
* state through the qualifier according to the setting in the parameter.
97106
*/
98107
class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
@@ -101,7 +110,10 @@ class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
101110
DisableDefaultEntityResolutionTranformer() {
102111
exists(Call call, Function f |
103112
call.getTarget() = f and
104-
f.getDeclaringType() instanceof AbstractDOMParserClass and
113+
(
114+
f.getDeclaringType() instanceof AbstractDOMParserClass or
115+
f.getDeclaringType() instanceof SAXParser
116+
) and
105117
f.hasName("setDisableDefaultEntityResolution") and
106118
this = call.getQualifier() and
107119
newValue = call.getArgument(0)
@@ -110,13 +122,13 @@ class DisableDefaultEntityResolutionTranformer extends XXEFlowStateTranformer {
110122

111123
final override XXEFlowState transform(XXEFlowState flowstate) {
112124
exists(int createEntityReferenceNodes |
113-
encodeXercesDOMFlowState(flowstate, _, createEntityReferenceNodes) and
125+
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
114126
(
115127
newValue.getValue().toInt() = 1 and // true
116-
encodeXercesDOMFlowState(result, 1, createEntityReferenceNodes)
128+
encodeXercesFlowState(result, 1, createEntityReferenceNodes)
117129
or
118130
not newValue.getValue().toInt() = 1 and // false or unknown
119-
encodeXercesDOMFlowState(result, 0, createEntityReferenceNodes)
131+
encodeXercesFlowState(result, 0, createEntityReferenceNodes)
120132
)
121133
)
122134
}
@@ -142,27 +154,31 @@ class CreateEntityReferenceNodesTranformer extends XXEFlowStateTranformer {
142154

143155
final override XXEFlowState transform(XXEFlowState flowstate) {
144156
exists(int disabledDefaultEntityResolution |
145-
encodeXercesDOMFlowState(flowstate, disabledDefaultEntityResolution, _) and
157+
encodeXercesFlowState(flowstate, disabledDefaultEntityResolution, _) and
146158
(
147159
newValue.getValue().toInt() = 1 and // true
148-
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 1)
160+
encodeXercesFlowState(result, disabledDefaultEntityResolution, 1)
149161
or
150162
not newValue.getValue().toInt() = 1 and // false or unknown
151-
encodeXercesDOMFlowState(result, disabledDefaultEntityResolution, 0)
163+
encodeXercesFlowState(result, disabledDefaultEntityResolution, 0)
152164
)
153165
)
154166
}
155167
}
156168

157169
/**
158-
* The `AbstractDOMParser.parse` method.
170+
* The `AbstractDOMParser.parse` or `SAXParser.parse` method.
159171
*/
160172
class ParseFunction extends Function {
161-
ParseFunction() { this.getClassAndName("parse") instanceof AbstractDOMParserClass }
173+
ParseFunction() {
174+
this.getClassAndName("parse") instanceof AbstractDOMParserClass or
175+
this.getClassAndName("parse") instanceof SAXParser
176+
}
162177
}
163178

164179
/**
165-
* The `createLSParser` function that returns a newly created `LSParser` object.
180+
* The `createLSParser` function that returns a newly created `DOMLSParser`
181+
* object.
166182
*/
167183
class CreateLSParser extends Function {
168184
CreateLSParser() {
@@ -184,14 +200,23 @@ class XXEConfiguration extends DataFlow::Configuration {
184200
call.getStaticCallTarget() = any(XercesDOMParserClass c).getAConstructor() and
185201
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
186202
call.getThisArgument() and
187-
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
203+
encodeXercesFlowState(flowstate, 0, 1) // default configuration
188204
)
189205
or
190206
// source is the result of a call to `createLSParser`.
191207
exists(Call call |
192208
call.getTarget() instanceof CreateLSParser and
193209
call = node.asExpr() and
194-
encodeXercesDOMFlowState(flowstate, 0, 1) // default configuration
210+
encodeXercesFlowState(flowstate, 0, 1) // default configuration
211+
)
212+
or
213+
// source is the write on `this` of a call to the `SAXParser`
214+
// constructor.
215+
exists(CallInstruction call |
216+
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
217+
call.getThisArgument() and
218+
call.getStaticCallTarget().(Constructor).getDeclaringType() instanceof SAXParser and
219+
encodeXercesFlowState(flowstate, 0, 1) // default configuration
195220
)
196221
}
197222

@@ -201,8 +226,8 @@ class XXEConfiguration extends DataFlow::Configuration {
201226
call.getTarget() instanceof ParseFunction and
202227
call.getQualifier() = node.asConvertedExpr()
203228
) and
204-
flowstate instanceof XercesDOMParserFlowState and
205-
not encodeXercesDOMFlowState(flowstate, 1, 1) // safe configuration
229+
flowstate instanceof XercesFlowState and
230+
not encodeXercesFlowState(flowstate, 1, 1) // safe configuration
206231
}
207232

208233
override predicate isAdditionalFlowStep(

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
11
edges
2+
| tests2.cpp:20:17:20:31 | SAXParser output argument | tests2.cpp:22:2:22:2 | p |
3+
| tests2.cpp:33:17:33:31 | SAXParser output argument | tests2.cpp:37:2:37:2 | p |
4+
| tests2.cpp:41:17:41:31 | SAXParser output argument | tests2.cpp:45:2:45:2 | p |
25
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p |
36
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p |
47
| tests.cpp:53:19:53:19 | VariableAddress [post update] | tests.cpp:55:2:55:2 | p |
@@ -27,6 +30,12 @@ edges
2730
| tests.cpp:146:18:146:18 | q | tests.cpp:134:39:134:39 | p |
2831
| tests.cpp:150:19:150:32 | call to createLSParser | tests.cpp:152:2:152:2 | p |
2932
nodes
33+
| tests2.cpp:20:17:20:31 | SAXParser output argument | semmle.label | SAXParser output argument |
34+
| tests2.cpp:22:2:22:2 | p | semmle.label | p |
35+
| tests2.cpp:33:17:33:31 | SAXParser output argument | semmle.label | SAXParser output argument |
36+
| tests2.cpp:37:2:37:2 | p | semmle.label | p |
37+
| tests2.cpp:41:17:41:31 | SAXParser output argument | semmle.label | SAXParser output argument |
38+
| tests2.cpp:45:2:45:2 | p | semmle.label | p |
3039
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
3140
| tests.cpp:35:2:35:2 | p | semmle.label | p |
3241
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
@@ -66,6 +75,9 @@ nodes
6675
| tests.cpp:152:2:152:2 | p | semmle.label | p |
6776
subpaths
6877
#select
78+
| 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 |
79+
| 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 |
80+
| tests2.cpp:45:2:45:2 | p | tests2.cpp:41:17:41:31 | SAXParser output argument | tests2.cpp:45:2:45:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests2.cpp:41:17:41:31 | SAXParser output argument | XML parser |
6981
| 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 |
7082
| 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 |
7183
| 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/tests2.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class SAXParser
1919
void test2_1(InputSource &data) {
2020
SAXParser *p = new SAXParser();
2121

22-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
22+
p->parse(data); // BAD (parser not correctly configured)
2323
}
2424

2525
void test2_2(InputSource &data) {
@@ -34,13 +34,13 @@ void test2_3(InputSource &data) {
3434
bool v = false;
3535

3636
p->setDisableDefaultEntityResolution(v);
37-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
37+
p->parse(data); // BAD (parser not correctly configured)
3838
}
3939

4040
void test2_4(InputSource &data) {
4141
SAXParser *p = new SAXParser();
4242
bool v = true;
4343

4444
p->setDisableDefaultEntityResolution(v);
45-
p->parse(data); // GOOD
45+
p->parse(data); // GOOD [FALSE POSITIVE]
4646
}

0 commit comments

Comments
 (0)