Skip to content

Commit 3dddc56

Browse files
committed
C++: Add LSParser specific transformer.
1 parent e3be774 commit 3dddc56

File tree

3 files changed

+76
-27
lines changed

3 files changed

+76
-27
lines changed

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,68 @@ class SetFeatureTranformer extends XXEFlowStateTranformer {
223223
}
224224
}
225225

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

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

Lines changed: 8 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,14 @@ edges
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 |
77
| tests5.cpp:27:25:27:38 | call to createLSParser | tests5.cpp:29:2:29:2 | p |
8-
| tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p |
98
| tests5.cpp:40:25:40:38 | call to createLSParser | tests5.cpp:43:2:43:2 | p |
10-
| tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p |
119
| tests5.cpp:55:25:55:38 | call to createLSParser | tests5.cpp:59:2:59:2 | p |
1210
| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:83:2:83:2 | p |
13-
| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86:2 | p |
14-
| tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:89:2:89:2 | p |
15-
| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p |
16-
| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p |
17-
| tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102: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 |
1816
| tests.cpp:15:23:15:43 | XercesDOMParser output argument | tests.cpp:17:2:17:2 | p |
1917
| tests.cpp:28:23:28:43 | XercesDOMParser output argument | tests.cpp:31:2:31:2 | p |
2018
| tests.cpp:35:19:35:19 | VariableAddress [post update] | tests.cpp:37:2:37:2 | p |
@@ -59,22 +57,17 @@ nodes
5957
| tests4.cpp:130:39:130:55 | (int)... | semmle.label | (int)... |
6058
| tests5.cpp:27:25:27:38 | call to createLSParser | semmle.label | call to createLSParser |
6159
| tests5.cpp:29:2:29:2 | p | semmle.label | p |
62-
| tests5.cpp:33:25:33:38 | call to createLSParser | semmle.label | call to createLSParser |
63-
| tests5.cpp:36:2:36:2 | p | semmle.label | p |
6460
| tests5.cpp:40:25:40:38 | call to createLSParser | semmle.label | call to createLSParser |
6561
| tests5.cpp:43:2:43:2 | p | semmle.label | p |
66-
| tests5.cpp:47:25:47:38 | call to createLSParser | semmle.label | call to createLSParser |
67-
| tests5.cpp:51:2:51:2 | p | semmle.label | p |
6862
| tests5.cpp:55:25:55:38 | call to createLSParser | semmle.label | call to createLSParser |
6963
| tests5.cpp:59:2:59:2 | p | semmle.label | p |
7064
| tests5.cpp:81:25:81:38 | call to createLSParser | semmle.label | call to createLSParser |
7165
| 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 |
7268
| tests5.cpp:86:2:86:2 | p | semmle.label | p |
69+
| tests5.cpp:88:2:88:2 | p | semmle.label | p |
7370
| tests5.cpp:89:2:89:2 | p | semmle.label | p |
74-
| tests5.cpp:93:25:93:38 | call to createLSParser | semmle.label | call to createLSParser |
75-
| tests5.cpp:96:2:96:2 | p | semmle.label | p |
76-
| tests5.cpp:99:2:99:2 | p | semmle.label | p |
77-
| tests5.cpp:102:2:102:2 | p | semmle.label | p |
7871
| tests.cpp:15:23:15:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
7972
| tests.cpp:17:2:17:2 | p | semmle.label | p |
8073
| tests.cpp:28:23:28:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
@@ -123,16 +116,10 @@ subpaths
123116
| 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 |
124117
| 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 |
125118
| 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 |
126-
| tests5.cpp:36:2:36:2 | p | tests5.cpp:33:25:33:38 | call to createLSParser | tests5.cpp:36:2:36:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:33:25:33:38 | call to createLSParser | XML parser |
127119
| 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 |
128-
| tests5.cpp:51:2:51:2 | p | tests5.cpp:47:25:47:38 | call to createLSParser | tests5.cpp:51:2:51:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:47:25:47:38 | call to createLSParser | XML parser |
129120
| 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 |
130121
| 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 |
131-
| tests5.cpp:86:2:86:2 | p | tests5.cpp:81:25:81:38 | call to createLSParser | tests5.cpp:86:2:86: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 |
132122
| 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 |
133-
| tests5.cpp:96:2:96:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:96:2:96:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser |
134-
| tests5.cpp:99:2:99:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:99:2:99:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser |
135-
| tests5.cpp:102:2:102:2 | p | tests5.cpp:93:25:93:38 | call to createLSParser | tests5.cpp:102:2:102:2 | p | This $@ is not configured to prevent an XML external entity (XXE) attack. | tests5.cpp:93:25:93:38 | call to createLSParser | XML parser |
136123
| 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 |
137124
| 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 |
138125
| 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: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ void test5_2(DOMImplementationLS *impl, InputSource &data) {
3333
DOMLSParser *p = impl->createLSParser();
3434

3535
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
36-
p->parse(data); // GOOD [FALSE POSITIVE]
36+
p->parse(data); // GOOD
3737
}
3838

3939
void test5_3(DOMImplementationLS *impl, InputSource &data) {
@@ -48,7 +48,7 @@ void test5_4(DOMImplementationLS *impl, InputSource &data) {
4848
DOMConfiguration *cfg = p->getDomConfig();
4949

5050
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
51-
p->parse(data); // GOOD [FALSE POSITIVE]
51+
p->parse(data); // GOOD
5252
}
5353

5454
void test5_5(DOMImplementationLS *impl, InputSource &data) {
@@ -83,7 +83,7 @@ void test5_7(DOMImplementationLS *impl, InputSource &data) {
8383
p->parse(data); // BAD (parser not correctly configured)
8484

8585
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
86-
p->parse(data); // GOOD [FALSE POSITIVE]
86+
p->parse(data); // GOOD
8787

8888
p->getDomConfig()->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
8989
p->parse(data); // BAD (parser not correctly configured)
@@ -93,11 +93,11 @@ void test5_8(DOMImplementationLS *impl, InputSource &data) {
9393
DOMLSParser *p = impl->createLSParser();
9494
DOMConfiguration *cfg = p->getDomConfig();
9595

96-
p->parse(data); // BAD (parser not correctly configured)
96+
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
9797

9898
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, true);
99-
p->parse(data); // GOOD [FALSE POSITIVE]
99+
p->parse(data); // GOOD
100100

101101
cfg->setParameter(XMLUni::fgXercesDisableDefaultEntityResolution, false);
102-
p->parse(data); // BAD (parser not correctly configured)
102+
p->parse(data); // BAD (parser not correctly configured) [NOT DETECTED]
103103
}

0 commit comments

Comments
 (0)