Skip to content

Commit 39551fd

Browse files
MathiasVPMathiasVP
authored
Merge pull request #9114 from geoffw0/xxe7
C++: Repair support for createLSParser in the CWE-611 XXE query.
2 parents 941485d + df30d22 commit 39551fd

File tree

3 files changed

+124
-6
lines changed

3 files changed

+124
-6
lines changed

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

Lines changed: 69 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ class XercesDOMParserClass extends Class {
5757
XercesDOMParserClass() { this.hasName("XercesDOMParser") }
5858
}
5959

60+
/**
61+
* The `DOMLSParser` class.
62+
*/
63+
class DomLSParserClass extends Class {
64+
DomLSParserClass() { this.hasName("DOMLSParser") }
65+
}
66+
6067
/**
6168
* The `SAXParser` class.
6269
*/
@@ -217,12 +224,71 @@ class SetFeatureTranformer extends XXEFlowStateTranformer {
217224
}
218225

219226
/**
220-
* The `AbstractDOMParser.parse`, `SAXParser.parse` or `SAX2XMLReader.parse`
221-
* method.
227+
* The `DOMLSParser.getDomConfig` function.
228+
*/
229+
class GetDomConfig extends Function {
230+
GetDomConfig() { this.getClassAndName("getDomConfig") instanceof DomLSParserClass }
231+
}
232+
233+
/**
234+
* The `DOMConfiguration.setParameter` function.
235+
*/
236+
class DomConfigurationSetParameter extends Function {
237+
DomConfigurationSetParameter() {
238+
this.getClassAndName("setParameter").getName() = "DOMConfiguration"
239+
}
240+
}
241+
242+
/**
243+
* A flow state transformer for a call to `DOMConfiguration.setParameter`
244+
* specifying the feature `XMLUni::fgXercesDisableDefaultEntityResolution`.
245+
* This is a slightly more complex transformer because the qualifier is a
246+
* `DOMConfiguration` pointer returned by `DOMLSParser.getDomConfig` - and it
247+
* is *that* qualifier we want to transform the flow state of.
248+
*/
249+
class DomConfigurationSetParameterTranformer extends XXEFlowStateTranformer {
250+
Expr newValue;
251+
252+
DomConfigurationSetParameterTranformer() {
253+
exists(FunctionCall getDomConfigCall, FunctionCall setParameterCall |
254+
// this is the qualifier of a call to `DOMLSParser.getDomConfig`.
255+
getDomConfigCall.getTarget() instanceof GetDomConfig and
256+
this = getDomConfigCall.getQualifier() and
257+
// `setParameterCall` is a call to `setParameter` on the return value of
258+
// the same call to `DOMLSParser.getDomConfig`.
259+
setParameterCall.getTarget() instanceof DomConfigurationSetParameter and
260+
globalValueNumber(setParameterCall.getQualifier()).getAnExpr() = getDomConfigCall and
261+
// the parameter being set is
262+
// `XMLUni::fgXercesDisableDefaultEntityResolution`.
263+
globalValueNumber(setParameterCall.getArgument(0)).getAnExpr().(VariableAccess).getTarget()
264+
instanceof FeatureDisableDefaultEntityResolution and
265+
// the value being set is `newValue`.
266+
newValue = setParameterCall.getArgument(1)
267+
)
268+
}
269+
270+
final override XXEFlowState transform(XXEFlowState flowstate) {
271+
exists(int createEntityReferenceNodes |
272+
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
273+
(
274+
globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // true
275+
encodeXercesFlowState(result, 1, createEntityReferenceNodes)
276+
or
277+
not globalValueNumber(newValue).getAnExpr().getValue().toInt() = 1 and // false or unknown
278+
encodeXercesFlowState(result, 0, createEntityReferenceNodes)
279+
)
280+
)
281+
}
282+
}
283+
284+
/**
285+
* The `AbstractDOMParser.parse`, `DOMLSParserClass.parse`, `SAXParser.parse`
286+
* or `SAX2XMLReader.parse` method.
222287
*/
223288
class ParseFunction extends Function {
224289
ParseFunction() {
225290
this.getClassAndName("parse") instanceof AbstractDOMParserClass or
291+
this.getClassAndName("parse") instanceof DomLSParserClass or
226292
this.getClassAndName("parse") instanceof SaxParserClass or
227293
this.getClassAndName("parse") instanceof Sax2XmlReader
228294
}
@@ -235,7 +301,7 @@ class ParseFunction extends Function {
235301
class CreateLSParser extends Function {
236302
CreateLSParser() {
237303
this.hasName("createLSParser") and
238-
this.getUnspecifiedType().(PointerType).getBaseType().getName() = "DOMLSParser" // returns a `DOMLSParser *`.
304+
this.getUnspecifiedType().(PointerType).getBaseType() instanceof DomLSParserClass // returns a `DOMLSParser *`.
239305
}
240306
}
241307

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

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,15 @@ edges
44
| tests3.cpp:23:21:23:53 | call to createXMLReader | tests3.cpp:25:2:25:2 | p |
55
| tests3.cpp:60:21:60:53 | call to createXMLReader | tests3.cpp:63:2:63:2 | p |
66
| tests3.cpp:67:21:67:53 | call to createXMLReader | tests3.cpp:70:2:70:2 | p |
7+
| tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p |
8+
| tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p |
9+
| tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p |
10+
| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p |
11+
| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p |
12+
| tests5.cpp:83:2:83:2 | p | tests5.cpp:85:2:85:2 | p |
13+
| tests5.cpp:85:2:85:2 | p | tests5.cpp:86:2:86:2 | p |
14+
| tests5.cpp:86:2:86:2 | p | tests5.cpp:88:2:88:2 | p |
15+
| tests5.cpp:88:2:88:2 | p | tests5.cpp:89:2:89:2 | p |
716
| tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p |
817
| tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p |
918
| tests.cpp:35:19:35:19 | VariableAddress [post update] | tests.cpp:37:2:37:2 | p |
@@ -46,6 +55,19 @@ nodes
4655
| tests4.cpp:46:34:46:68 | ... \| ... | semmle.label | ... \| ... |
4756
| tests4.cpp:77:34:77:38 | flags | semmle.label | flags |
4857
| tests4.cpp:130:39:130:55 | (int)... | semmle.label | (int)... |
58+
| tests5.cpp:27:25:27:38 | call to createLSParser | semmle.label | call to createLSParser |
59+
| tests5.cpp:29:2:29:2 | p | semmle.label | p |
60+
| tests5.cpp:40:25:40:38 | call to createLSParser | semmle.label | call to createLSParser |
61+
| tests5.cpp:43:2:43:2 | p | semmle.label | p |
62+
| tests5.cpp:55:25:55:38 | call to createLSParser | semmle.label | call to createLSParser |
63+
| tests5.cpp:59:2:59:2 | p | semmle.label | p |
64+
| tests5.cpp:81:25:81:38 | call to createLSParser | semmle.label | call to createLSParser |
65+
| tests5.cpp:83:2:83:2 | p | semmle.label | p |
66+
| tests5.cpp:83:2:83:2 | p | semmle.label | p |
67+
| tests5.cpp:85:2:85:2 | p | semmle.label | p |
68+
| tests5.cpp:86:2:86:2 | p | semmle.label | p |
69+
| tests5.cpp:88:2:88:2 | p | semmle.label | p |
70+
| tests5.cpp:89:2:89:2 | p | semmle.label | p |
4971
| tests.cpp:15:23:15:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
5072
| tests.cpp:17:2:17:2 | p | semmle.label | p |
5173
| tests.cpp:28:23:28:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
@@ -93,6 +115,11 @@ subpaths
93115
| 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 |
94116
| 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 |
95117
| 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 |
118+
| tests5.cpp:29:2:29:2 | p | tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:27:25:27:38 | call to createLSParser | XML parser |
119+
| tests5.cpp:43:2:43:2 | p | tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:40:25:40:38 | call to createLSParser | XML parser |
120+
| tests5.cpp:59:2:59:2 | p | tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:55:25:55:38 | call to createLSParser | XML parser |
121+
| tests5.cpp:83:2:83:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser |
122+
| tests5.cpp:89:2:89:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:81:25:81:38 | call to createLSParser | XML parser |
96123
| tests.cpp:17:2:17:2 | p | tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:15:23:15:43 | XercesDOMParser output argument | XML parser |
97124
| tests.cpp:31:2:31:2 | p | tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:28:23:28:43 | XercesDOMParser output argument | XML parser |
98125
| tests.cpp:39:2:39:2 | p | tests.cpp:35:23:35:43 | XercesDOMParser output argument | tests.cpp:39:2:39:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests.cpp:35:23:35:43 | XercesDOMParser output argument | XML parser |

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class DOMImplementationLS {
2626
void test5_1(DOMImplementationLS *impl, InputSource &data) {
2727
DOMLSParser *p = impl->createLSParser();
2828

29-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
29+
p->parse(data); // BAD (parser not correctly configured)
3030
}
3131

3232
void test5_2(DOMImplementationLS *impl, InputSource &data) {
@@ -40,7 +40,7 @@ void test5_3(DOMImplementationLS *impl, InputSource &data) {
4040
DOMLSParser *p = impl->createLSParser();
4141

4242
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
43-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
43+
p->parse(data); // BAD (parser not correctly configured)
4444
}
4545

4646
void test5_4(DOMImplementationLS *impl, InputSource &data) {
@@ -56,7 +56,7 @@ void test5_5(DOMImplementationLS *impl, InputSource &data) {
5656
DOMConfiguration *cfg = p->getDomConfig();
5757

5858
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
59-
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
59+
p->parse(data); // BAD (parser not correctly configured)
6060
}
6161

6262
DOMImplementationLS *g_impl;
@@ -76,3 +76,28 @@ void test5_6() {
7676
g_p1->parse(*g_data); // GOOD
7777
g_p2->parse(*g_data); // BAD (parser not correctly configured) [NOT DETECTED]
7878
}
79+
80+
void test5_7(DOMImplementationLS *impl, InputSource &data) {
81+
DOMLSParser *p = impl->createLSParser();
82+
83+
p->parse(data); // BAD (parser not correctly configured)
84+
85+
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
86+
p->parse(data); // GOOD
87+
88+
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
89+
p->parse(data); // BAD (parser not correctly configured)
90+
}
91+
92+
void test5_8(DOMImplementationLS *impl, InputSource &data) {
93+
DOMLSParser *p = impl->createLSParser();
94+
DOMConfiguration *cfg = p->getDomConfig();
95+
96+
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
97+
98+
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
99+
p->parse(data); // GOOD
100+
101+
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
102+
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
103+
}

0 commit comments

Comments
 (0)