Skip to content

Commit 6b5a192

Browse files
geoffw0geoffw0
committed
C++: Support the SAX2XMLReader interface.
1 parent c4bc705 commit 6b5a192

File tree

3 files changed

+86
-4
lines changed

3 files changed

+86
-4
lines changed

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

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,13 @@ class SAXParserClass extends Class {
6464
SAXParserClass() { this.hasName("SAXParser") }
6565
}
6666

67+
/**
68+
* The `SAX2XMLReader` class.
69+
*/
70+
class SAX2XMLReader extends Class {
71+
SAX2XMLReader() { this.hasName("SAX2XMLReader") }
72+
}
73+
6774
/**
6875
* Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
6976
*
@@ -168,12 +175,57 @@ class CreateEntityReferenceNodesTranformer extends XXEFlowStateTranformer {
168175
}
169176

170177
/**
171-
* The `AbstractDOMParser.parse` or `SAXParser.parse` method.
178+
* The `XMLUni.fgXercesDisableDefaultEntityResolution` constant.
179+
*/
180+
class FeatureDisableDefaultEntityResolution extends Variable {
181+
FeatureDisableDefaultEntityResolution() {
182+
this.getName() = "fgXercesDisableDefaultEntityResolution" and
183+
this.getDeclaringType().getName() = "XMLUni"
184+
}
185+
}
186+
187+
/**
188+
* A flow state transformer for a call to `SAX2XMLReader.setFeature(XMLUni::fgXercesDisableDefaultEntityResolution, *)`. Transforms the flow
189+
* state through the qualifier according to this setting.
190+
*/
191+
class SetFeatureTranformer extends XXEFlowStateTranformer {
192+
Expr newValue;
193+
194+
SetFeatureTranformer() {
195+
exists(Call call, Function f |
196+
call.getTarget() = f and
197+
f.getDeclaringType() instanceof SAX2XMLReader and
198+
f.hasName("setFeature") and
199+
this = call.getQualifier() and
200+
globalValueNumber(call.getArgument(0)).getAnExpr().(VariableAccess).getTarget() instanceof
201+
FeatureDisableDefaultEntityResolution and
202+
newValue = call.getArgument(1)
203+
)
204+
}
205+
206+
final override XXEFlowState transform(XXEFlowState flowstate) {
207+
exists(int createEntityReferenceNodes |
208+
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
209+
(
210+
globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // true
211+
encodeXercesFlowState(result, 1, createEntityReferenceNodes)
212+
or
213+
not globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // false or unknown
214+
encodeXercesFlowState(result, 0, createEntityReferenceNodes)
215+
)
216+
)
217+
}
218+
}
219+
220+
/**
221+
* The `AbstractDOMParser.parse`, `SAXParser.parse` or `SAX2XMLReader.parse`
222+
* method.
172223
*/
173224
class ParseFunction extends Function {
174225
ParseFunction() {
175226
this.getClassAndName("parse") instanceof AbstractDOMParserClass or
176-
this.getClassAndName("parse") instanceof SAXParserClass
227+
this.getClassAndName("parse") instanceof SAXParserClass or
228+
this.getClassAndName("parse") instanceof SAX2XMLReader
177229
}
178230
}
179231

@@ -188,6 +240,17 @@ class CreateLSParser extends Function {
188240
}
189241
}
190242

243+
/**
244+
* The `createXMLReader` function that returns a newly created `SAX2XMLReader`
245+
* object.
246+
*/
247+
class CreateXMLReader extends Function {
248+
CreateXMLReader() {
249+
this.hasName("createXMLReader") and
250+
this.getUnspecifiedType().(PointerType).getBaseType() instanceof SAX2XMLReader // returns a `SAX2XMLReader *`.
251+
}
252+
}
253+
191254
/**
192255
* A call to a `libxml2` function that parses XML.
193256
*/
@@ -256,6 +319,13 @@ class XXEConfiguration extends DataFlow::Configuration {
256319
encodeXercesFlowState(flowstate, 0, 1) // default configuration
257320
)
258321
or
322+
// source is the result of a call to `createXMLReader`.
323+
exists(Call call |
324+
call.getTarget() instanceof CreateXMLReader and
325+
call = node.asExpr() and
326+
encodeXercesFlowState(flowstate, 0, 1) // default configuration
327+
)
328+
or
259329
// source is an `options` argument on a `libxml2` parse call that specifies
260330
// at least one unsafe option.
261331
//

‎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,6 +1,9 @@
11
edges
22
| tests2.cpp:20:17:20:31 | SAXParser output argument | tests2.cpp:22:2:22:2 | p |
33
| tests2.cpp:33:17:33:31 | SAXParser output argument | tests2.cpp:37:2:37:2 | p |
4+
| tests3.cpp:23:21:23:53 | call to createXMLReader | tests3.cpp:25:2:25:2 | p |
5+
| tests3.cpp:60:21:60:53 | call to createXMLReader | tests3.cpp:63:2:63:2 | p |
6+
| tests3.cpp:67:21:67:53 | call to createXMLReader | tests3.cpp:70:2:70:2 | p |
47
| tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p |
58
| tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p |
69
| tests.cpp:35:19:35:19 | VariableAddress [post update] | tests.cpp:37:2:37:2 | p |
@@ -32,6 +35,12 @@ nodes
3235
| tests2.cpp:22:2:22:2 | p | semmle.label | p |
3336
| tests2.cpp:33:17:33:31 | SAXParser output argument | semmle.label | SAXParser output argument |
3437
| tests2.cpp:37:2:37:2 | p | semmle.label | p |
38+
| tests3.cpp:23:21:23:53 | call to createXMLReader | semmle.label | call to createXMLReader |
39+
| tests3.cpp:25:2:25:2 | p | semmle.label | p |
40+
| tests3.cpp:60:21:60:53 | call to createXMLReader | semmle.label | call to createXMLReader |
41+
| tests3.cpp:63:2:63:2 | p | semmle.label | p |
42+
| tests3.cpp:67:21:67:53 | call to createXMLReader | semmle.label | call to createXMLReader |
43+
| tests3.cpp:70:2:70:2 | p | semmle.label | p |
3544
| tests4.cpp:26:34:26:48 | (int)... | semmle.label | (int)... |
3645
| tests4.cpp:36:34:36:50 | (int)... | semmle.label | (int)... |
3746
| tests4.cpp:46:34:46:68 | ... \| ... | semmle.label | ... \| ... |
@@ -76,6 +85,9 @@ subpaths
7685
#select
7786
| 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 |
7887
| 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 |
88+
| tests3.cpp:25:2:25:2 | p | tests3.cpp:23:21:23:53 | call to createXMLReader | tests3.cpp:25:2:25:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests3.cpp:23:21:23:53 | call to createXMLReader | XML parser |
89+
| tests3.cpp:63:2:63:2 | p | tests3.cpp:60:21:60:53 | call to createXMLReader | tests3.cpp:63:2:63:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests3.cpp:60:21:60:53 | call to createXMLReader | XML parser |
90+
| tests3.cpp:70:2:70:2 | p | tests3.cpp:67:21:67:53 | call to createXMLReader | tests3.cpp:70:2:70:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests3.cpp:67:21:67:53 | call to createXMLReader | XML parser |
7991
| 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 |
8092
| 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 |
8193
| 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 |

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,14 @@ void test3_6(InputSource &data) {
6060
SAX2XMLReader *p = XMLReaderFactory::createXMLReader();
6161

6262
p->setFeature(XMLUni::fgXercesDisableDefaultEntityResolution, false);
63-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
63+
p->parse(data); // BAD (parser not correctly configured)
6464
}
6565

6666
void test3_7(InputSource &data) {
6767
SAX2XMLReader *p = XMLReaderFactory::createXMLReader();
6868

6969
p->setFeature(XMLUni::fgXercesHarmlessOption, true);
70-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
70+
p->parse(data); // BAD (parser not correctly configured)
7171
}
7272

7373
void test3_8(InputSource &data) {

0 commit comments

Comments
 (0)