Skip to content

Commit 3aaa058

Browse files
geoffw0geoffw0
committed
C++: Get the simplest part of the query working, disable the rest for now, fix metadata, formatting etc.
1 parent 9a0880f commit 3aaa058

3 files changed

Lines changed: 146 additions & 42 deletions

File tree

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

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
/**
22
* @name External Entity Expansion
3-
* @description
3+
* @description TODO
44
* @kind path-problem
55
* @id cpp/external-entity-expansion
66
* @problem.severity warning
7+
* @security-severity TODO
8+
* @precision TODO
79
* @tags security
810
* external/cwe/cwe-611
911
*/
@@ -13,74 +15,92 @@ import semmle.code.cpp.ir.dataflow.DataFlow
1315
import DataFlow::PathGraph
1416
import semmle.code.cpp.ir.IR
1517

16-
class XercesDOMParser extends Class {
17-
XercesDOMParser() { this.hasName("XercesDOMParser") }
18-
}
19-
2018
class AbstractDOMParser extends Class {
2119
AbstractDOMParser() { this.hasName("AbstractDOMParser") }
2220
}
2321

22+
class XercesDOMParser extends Class {
23+
XercesDOMParser() { this.hasName("XercesDOMParser") }
24+
}
25+
2426
class DisableDefaultEntityResolution extends Function {
2527
DisableDefaultEntityResolution() {
26-
this.hasQualifiedName(_, "AbstractOMParser", "setDisableDefaultEntityResolution")
28+
this.getDeclaringType() instanceof AbstractDOMParser and
29+
this.hasName("setDisableDefaultEntityResolution")
2730
}
2831
}
2932

3033
class SetCreateEntityReferenceNodes extends Function {
3134
SetCreateEntityReferenceNodes() {
32-
this.hasQualifiedName(_, "AbstractDOMParser", "setCreateEntityReferenceNodes")
35+
this.getDeclaringType() instanceof AbstractDOMParser and
36+
this.hasName("setCreateEntityReferenceNodes")
3337
}
3438
}
3539

36-
class CreateLSParser extends Function {
37-
CreateLSParser() {
38-
this.hasName("createLSParser")
40+
class Parse extends Function {
41+
Parse() {
42+
this.getDeclaringType() instanceof AbstractDOMParser and
43+
this.hasName("parse")
3944
}
4045
}
4146

47+
/*
48+
class CreateLSParser extends Function {
49+
CreateLSParser() { this.hasName("createLSParser") }
50+
}
51+
4252
class SetSecurityManager extends Function {
43-
SetSecurityManager() {
44-
this.hasQualifiedName(_, "AbstractDOMParser", "setSecurityManager")
45-
}
53+
SetSecurityManager() { this.hasQualifiedName(_, "AbstractDOMParser", "setSecurityManager") }
4654
}
4755
4856
class SAXParser extends Class {
4957
SAXParser() { this.hasName("SAXParser") }
5058
}
51-
59+
*/
5260
class XercesXXEConfiguration extends DataFlow::Configuration {
5361
XercesXXEConfiguration() { this = "XercesXXEConfiguration" }
5462

55-
override predicate isSource(DataFlow::Node node, string flowstate) {
63+
override predicate isSource(DataFlow::Node node/*, string flowstate*/) {
64+
// source is the write on `this` of a call to the XercesDOMParser
65+
// constructor.
5666
exists(CallInstruction call |
57-
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() = call.getThisArgument() and
58-
call.getStaticCallTarget().(Constructor).getDeclaringType() instanceof XercesDOMParser and
59-
flowstate = "XercesDOM"
67+
call.getStaticCallTarget() = any(XercesDOMParser c).getAConstructor() and
68+
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
69+
call.getThisArgument()/* and
70+
flowstate = "XercesDOM"*/
6071
)
61-
or
72+
/*exists(Call call |
73+
call.getTarget() = any(XercesDOMParser c).getAConstructor() and
74+
node.asExpr() = call
75+
)*/
76+
/* or
6277
exists(Call call |
6378
call.getTarget() instanceof CreateLSParser and
6479
call = node.asExpr() and
6580
flowstate = "XercesDOM"
6681
)
6782
or
6883
exists(CallInstruction call |
69-
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() = call.getThisArgument() and
84+
node.asInstruction().(WriteSideEffectInstruction).getDestinationAddress() =
85+
call.getThisArgument() and
7086
call.getStaticCallTarget().(Constructor).getDeclaringType() instanceof SAXParser and
7187
flowstate = "SAXParser"
72-
)
88+
)*/
7389
}
7490

7591
override predicate isSink(DataFlow::Node node) {
76-
exists(Call call, ReadSideEffectInstruction instr |
77-
call.getTarget().hasName("parse") and
78-
call.getQualifier() = instr.getArgumentDef().getUnconvertedResultExpression() and
79-
node.asOperand() = instr.getSideEffectOperand()
92+
// sink is the read of the qualifier of a call to `parse`.
93+
exists(Call call/*, ReadSideEffectInstruction instr*/ |
94+
call.getTarget() instanceof Parse and
95+
call.getQualifier() = node.asConvertedExpr()
96+
/*instr.getArgumentDef().getUnconvertedResultExpression() and
97+
node.asOperand() = instr.getSideEffectOperand()*/
8098
)
8199
}
82100

83-
override predicate isAdditionalFlowStep(DataFlow::Node node1, string state1, DataFlow::Node node2, string state2) {
101+
/*override predicate isAdditionalFlowStep(
102+
DataFlow::Node node1, string state1, DataFlow::Node node2, string state2
103+
) {
84104
exists(Call call |
85105
node1.asConvertedExpr() = call.getQualifier() and
86106
node2.asDefiningArgument() = call.getQualifier() and
@@ -94,9 +114,9 @@ class XercesXXEConfiguration extends DataFlow::Configuration {
94114
state2 = "XercesDOM-SCERN"
95115
)
96116
)
97-
}
117+
}*/
98118

99-
override predicate isBarrier(DataFlow::Node node, string flowstate) {
119+
/*override predicate isBarrier(DataFlow::Node node, string flowstate) {
100120
exists(Call call |
101121
(
102122
flowstate = "XercesDOM-DDER" and
@@ -110,14 +130,15 @@ class XercesXXEConfiguration extends DataFlow::Configuration {
110130
or
111131
exists(Call setSecurityManager |
112132
// todo: security manager setup
133+
flowstate = TODO
113134
setSecurityManager.getQualifier() = node.asDefiningArgument() and
114135
setSecurityManager.getTarget() instanceof SetSecurityManager
115136
)
116-
//or
117-
}
137+
}*/
118138
}
119139

120140
/*
141+
* TODO:
121142
* parser created
122143
* needs doSchema set?
123144
* needs validation set?
@@ -128,7 +149,7 @@ class XercesXXEConfiguration extends DataFlow::Configuration {
128149
* no
129150
*/
130151

131-
from DataFlow::PathNode source, DataFlow::PathNode sink, XercesXXEConfiguration conf
152+
from XercesXXEConfiguration conf, DataFlow::PathNode source, DataFlow::PathNode sink
132153
where conf.hasFlowPath(source, sink)
133154
select sink, source, sink,
134-
"This $@ is not configured to prevent an External Entity Expansion attack.", source, "XML parser"
155+
"This $@ is not configured to prevent an External Entity Expansion (XXE) attack.", source, "XML parser"

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

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,87 @@
11
edges
2+
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | tests.cpp:35:2:35:2 | p |
3+
| tests.cpp:39:23:39:43 | XercesDOMParser output argument | tests.cpp:42:2:42:2 | p |
4+
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | tests.cpp:49:2:49:2 | p |
5+
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | tests.cpp:57:2:57:2 | p |
6+
| tests.cpp:61:23:61:43 | XercesDOMParser output argument | tests.cpp:65:2:65:2 | p |
7+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:72:2:72:2 | p |
8+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:74:2:74:2 | p |
9+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:76:2:76:2 | p |
10+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:78:2:78:2 | p |
11+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:80:2:80:2 | p |
12+
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p |
13+
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p |
14+
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q |
15+
| tests.cpp:110:24:110:44 | XercesDOMParser output argument | tests.cpp:114:3:114:3 | q |
16+
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | tests.cpp:122:3:122:3 | q |
17+
| tests.cpp:126:39:126:39 | p | tests.cpp:127:2:127:2 | p |
18+
| tests.cpp:130:39:130:39 | p | tests.cpp:131:2:131:2 | p |
19+
| tests.cpp:134:39:134:39 | p | tests.cpp:135:2:135:2 | p |
20+
| tests.cpp:139:23:139:43 | XercesDOMParser output argument | tests.cpp:143:18:143:18 | p |
21+
| tests.cpp:139:23:139:43 | XercesDOMParser output argument | tests.cpp:145:18:145:18 | p |
22+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:144:18:144:18 | q |
23+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | tests.cpp:146:18:146:18 | q |
24+
| tests.cpp:143:18:143:18 | p | tests.cpp:126:39:126:39 | p |
25+
| tests.cpp:144:18:144:18 | q | tests.cpp:130:39:130:39 | p |
26+
| tests.cpp:145:18:145:18 | p | tests.cpp:134:39:134:39 | p |
27+
| tests.cpp:146:18:146:18 | q | tests.cpp:134:39:134:39 | p |
228
nodes
29+
| tests.cpp:33:23:33:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
30+
| tests.cpp:35:2:35:2 | p | semmle.label | p |
31+
| tests.cpp:39:23:39:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
32+
| tests.cpp:42:2:42:2 | p | semmle.label | p |
33+
| tests.cpp:46:23:46:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
34+
| tests.cpp:49:2:49:2 | p | semmle.label | p |
35+
| tests.cpp:53:23:53:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
36+
| tests.cpp:57:2:57:2 | p | semmle.label | p |
37+
| tests.cpp:61:23:61:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
38+
| tests.cpp:65:2:65:2 | p | semmle.label | p |
39+
| tests.cpp:69:23:69:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
40+
| tests.cpp:72:2:72:2 | p | semmle.label | p |
41+
| tests.cpp:74:2:74:2 | p | semmle.label | p |
42+
| tests.cpp:76:2:76:2 | p | semmle.label | p |
43+
| tests.cpp:78:2:78:2 | p | semmle.label | p |
44+
| tests.cpp:80:2:80:2 | p | semmle.label | p |
45+
| tests.cpp:84:23:84:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
46+
| tests.cpp:87:2:87:2 | p | semmle.label | p |
47+
| tests.cpp:91:23:91:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
48+
| tests.cpp:98:2:98:2 | p | semmle.label | p |
49+
| tests.cpp:103:24:103:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
50+
| tests.cpp:106:3:106:3 | q | semmle.label | q |
51+
| tests.cpp:110:24:110:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
52+
| tests.cpp:114:3:114:3 | q | semmle.label | q |
53+
| tests.cpp:118:24:118:44 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
54+
| tests.cpp:122:3:122:3 | q | semmle.label | q |
55+
| tests.cpp:126:39:126:39 | p | semmle.label | p |
56+
| tests.cpp:127:2:127:2 | p | semmle.label | p |
57+
| tests.cpp:130:39:130:39 | p | semmle.label | p |
58+
| tests.cpp:131:2:131:2 | p | semmle.label | p |
59+
| tests.cpp:134:39:134:39 | p | semmle.label | p |
60+
| tests.cpp:135:2:135:2 | p | semmle.label | p |
61+
| tests.cpp:139:23:139:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
62+
| tests.cpp:140:23:140:43 | XercesDOMParser output argument | semmle.label | XercesDOMParser output argument |
63+
| tests.cpp:143:18:143:18 | p | semmle.label | p |
64+
| tests.cpp:144:18:144:18 | q | semmle.label | q |
65+
| tests.cpp:145:18:145:18 | p | semmle.label | p |
66+
| tests.cpp:146:18:146:18 | q | semmle.label | q |
367
subpaths
468
#select
69+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:33:23:33:43 | XercesDOMParser output argument | XML parser |
70+
| tests.cpp:42:2:42:2 | p | tests.cpp:39:23:39:43 | XercesDOMParser output argument | tests.cpp:42:2:42:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:39:23:39:43 | XercesDOMParser output argument | XML parser |
71+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:46:23:46:43 | XercesDOMParser output argument | XML parser |
72+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:53:23:53:43 | XercesDOMParser output argument | XML parser |
73+
| tests.cpp:65:2:65:2 | p | tests.cpp:61:23:61:43 | XercesDOMParser output argument | tests.cpp:65:2:65:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:61:23:61:43 | XercesDOMParser output argument | XML parser |
74+
| tests.cpp:72:2:72:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:72:2:72:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
75+
| tests.cpp:74:2:74:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:74:2:74:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
76+
| tests.cpp:76:2:76:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:76:2:76:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
77+
| tests.cpp:78:2:78:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:78:2:78:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
78+
| tests.cpp:80:2:80:2 | p | tests.cpp:69:23:69:43 | XercesDOMParser output argument | tests.cpp:80:2:80:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:69:23:69:43 | XercesDOMParser output argument | XML parser |
79+
| tests.cpp:87:2:87:2 | p | tests.cpp:84:23:84:43 | XercesDOMParser output argument | tests.cpp:87:2:87:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:84:23:84:43 | XercesDOMParser output argument | XML parser |
80+
| tests.cpp:98:2:98:2 | p | tests.cpp:91:23:91:43 | XercesDOMParser output argument | tests.cpp:98:2:98:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:91:23:91:43 | XercesDOMParser output argument | XML parser |
81+
| tests.cpp:106:3:106:3 | q | tests.cpp:103:24:103:44 | XercesDOMParser output argument | tests.cpp:106:3:106:3 | q | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:103:24:103:44 | XercesDOMParser output argument | XML parser |
82+
| tests.cpp:114:3:114:3 | q | tests.cpp:110:24:110:44 | XercesDOMParser output argument | tests.cpp:114:3:114:3 | q | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:110:24:110:44 | XercesDOMParser output argument | XML parser |
83+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:118:24:118:44 | XercesDOMParser output argument | XML parser |
84+
| tests.cpp:127:2:127:2 | p | tests.cpp:139:23:139:43 | XercesDOMParser output argument | tests.cpp:127:2:127:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:139:23:139:43 | XercesDOMParser output argument | XML parser |
85+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |
86+
| tests.cpp:135:2:135:2 | p | tests.cpp:139:23:139:43 | XercesDOMParser output argument | tests.cpp:135:2:135:2 | p | This $@ is not configured to prevent an External Entity Expansion (XXE) attack. | tests.cpp:139:23:139:43 | XercesDOMParser output argument | XML parser |
87+
| 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 External Entity Expansion (XXE) attack. | tests.cpp:140:23:140:43 | XercesDOMParser output argument | XML parser |

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ void test2(InputSource &data) {
3939
XercesDOMParser *p = new XercesDOMParser();
4040

4141
p->setDisableDefaultEntityResolution(true);
42-
p->parse(data); // GOOD
42+
p->parse(data); // GOOD [FALSE POSITIVE]
4343
}
4444

4545
void test3(InputSource &data) {
@@ -62,22 +62,22 @@ void test5(InputSource &data) {
6262

6363
p->setDisableDefaultEntityResolution(true);
6464
p->setCreateEntityReferenceNodes(true);
65-
p->parse(data); // GOOD
65+
p->parse(data); // GOOD [FALSE POSITIVE]
6666
}
6767

6868
void test6(InputSource &data) {
6969
XercesDOMParser *p = new XercesDOMParser();
7070

7171
p->setDisableDefaultEntityResolution(true);
72-
p->parse(data); // GOOD
72+
p->parse(data); // GOOD [FALSE POSITIVE]
7373
p->setDisableDefaultEntityResolution(false);
7474
p->parse(data); // BAD (parser not correctly configured)
7575
p->setDisableDefaultEntityResolution(true);
76-
p->parse(data); // GOOD
76+
p->parse(data); // GOOD [FALSE POSITIVE]
7777
p->setCreateEntityReferenceNodes(false);
7878
p->parse(data); // BAD (parser not correctly configured)
7979
p->setCreateEntityReferenceNodes(true);
80-
p->parse(data); // GOOD
80+
p->parse(data); // GOOD [FALSE POSITIVE]
8181
}
8282

8383
void test7(InputSource &data, bool cond) {
@@ -111,20 +111,20 @@ void test9(InputSource &data) {
111111
XercesDOMParser &q = *p;
112112

113113
q.setDisableDefaultEntityResolution(true);
114-
q.parse(data); // GOOD
114+
q.parse(data); // GOOD [FALSE POSITIVE]
115115
}
116116

117117
{
118118
XercesDOMParser *p = new XercesDOMParser();
119119
XercesDOMParser &q = *p;
120120

121121
p->setDisableDefaultEntityResolution(true);
122-
q.parse(data); // GOOD
122+
q.parse(data); // GOOD [FALSE POSITIVE]
123123
}
124124
}
125125

126126
void test10_doParseA(XercesDOMParser *p, InputSource &data) {
127-
p->parse(data); // GOOD
127+
p->parse(data); // GOOD [FALSE POSITIVE]
128128
}
129129

130130
void test10_doParseB(XercesDOMParser *p, InputSource &data) {
@@ -149,7 +149,7 @@ void test10(InputSource &data) {
149149
void test11(InputSource &data) {
150150
LSParser *p = createLSParser();
151151

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

155155
void test12(InputSource &data) {
@@ -166,5 +166,5 @@ InputSource *g_data;
166166
void test13() {
167167
g_p1->setDisableDefaultEntityResolution(true);
168168
g_p1->parse(*g_data); // GOOD
169-
g_p2->parse(*g_data); // BAD (parser not correctly configured)
169+
g_p2->parse(*g_data); // BAD (parser not correctly configured) [NOT DETECTED]
170170
}

0 commit comments

Comments
 (0)