Skip to content

Commit 30cbc91

Browse files
jketemajketema
committed
C++: Update XXE XML query with DataFlow::ConfigSig 
1 parent 6f24074 commit 30cbc91

File tree

4 files changed

+66
-60
lines changed

4 files changed

+66
-60
lines changed

‎cpp/ql/src/Security/CWE/CWE-611/Libxml2.qll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class Libxml2BadOption extends EnumConstant {
4848
class LibXml2Library extends XmlLibrary {
4949
LibXml2Library() { this = "LibXml2Library" }
5050

51-
override predicate configurationSource(DataFlow::Node node, string flowstate) {
51+
override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
5252
// source is an `options` argument on a libxml2 parse call that specifies
5353
// at least one unsafe option.
5454
//
@@ -59,20 +59,20 @@ class LibXml2Library extends XmlLibrary {
5959
exists(Libxml2ParseCall call, Expr options |
6060
options = call.getOptions() and
6161
node.asExpr() = options and
62-
flowstate = "libxml2" and
62+
flowstate instanceof TLibXml2FlowState and
6363
exists(Libxml2BadOption opt |
6464
globalValueNumber(options).getAnExpr().getValue().toInt().bitAnd(opt.getValue().toInt()) !=
6565
0
6666
)
6767
)
6868
}
6969

70-
override predicate configurationSink(DataFlow::Node node, string flowstate) {
70+
override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
7171
// sink is the `options` argument on a `libxml2` parse call.
7272
exists(Libxml2ParseCall call, Expr options |
7373
options = call.getOptions() and
7474
node.asExpr() = options and
75-
flowstate = "libxml2"
75+
flowstate instanceof TLibXml2FlowState
7676
)
7777
}
7878
}

‎cpp/ql/src/Security/CWE/CWE-611/XML.qll

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,20 @@ import Libxml2
1111
/**
1212
* A flow state representing a possible configuration of an XML object.
1313
*/
14-
abstract class XxeFlowState extends DataFlow::FlowState {
15-
bindingset[this]
16-
XxeFlowState() { any() } // required characteristic predicate
17-
}
14+
newtype TXxeFlowState =
15+
/**
16+
* Flow state for `AbstractDOMParser` or `SAXParser`, where:
17+
* - `disabledDefaultEntityResolution` is 1 if `setDisableDefaultEntityResolution`
18+
* is `true`, 0 otherwise.
19+
* - `createEntityReferenceNodes` is 1 if `setCreateEntityReferenceNodes`
20+
* is `true`, 0 otherwise.
21+
*/
22+
TXercesFlowState(int disabledDefaultEntityResolution, int createEntityReferenceNodes) {
23+
disabledDefaultEntityResolution in [0, 1] and
24+
createEntityReferenceNodes in [0, 1]
25+
} or
26+
/** Flow state for libxml2 */
27+
TLibXml2FlowState()
1828

1929
/**
2030
* An XML library or interface.
@@ -28,13 +38,13 @@ abstract class XmlLibrary extends string {
2838
* object for this XML library, along with `flowstate` representing its
2939
* initial state.
3040
*/
31-
abstract predicate configurationSource(DataFlow::Node node, string flowstate);
41+
abstract predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate);
3242

3343
/**
3444
* Holds if `node` is the sink node where an unsafe configuration object is
3545
* used to interpret XML.
3646
*/
37-
abstract predicate configurationSink(DataFlow::Node node, string flowstate);
47+
abstract predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate);
3848
}
3949

4050
/**
@@ -51,5 +61,5 @@ abstract class XxeFlowStateTransformer extends Expr {
5161
* transform(transform(x)) = transform(x)
5262
* ```
5363
*/
54-
abstract XxeFlowState transform(XxeFlowState flowstate);
64+
abstract TXxeFlowState transform(TXxeFlowState flowstate);
5565
}

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,38 +14,40 @@
1414

1515
import cpp
1616
import XML
17-
import DataFlow::PathGraph
17+
import XxeFlow::PathGraph
1818

1919
/**
2020
* A configuration for tracking XML objects and their states.
2121
*/
22-
class XxeConfiguration extends DataFlow::Configuration {
23-
XxeConfiguration() { this = "XXEConfiguration" }
22+
module XxeConfiguration implements DataFlow::StateConfigSig {
23+
class FlowState = TXxeFlowState;
2424

25-
override predicate isSource(DataFlow::Node node, string flowstate) {
25+
predicate isSource(DataFlow::Node node, FlowState flowstate) {
2626
any(XmlLibrary l).configurationSource(node, flowstate)
2727
}
2828

29-
override predicate isSink(DataFlow::Node node, string flowstate) {
29+
predicate isSink(DataFlow::Node node, FlowState flowstate) {
3030
any(XmlLibrary l).configurationSink(node, flowstate)
3131
}
3232

33-
override predicate isAdditionalFlowStep(
34-
DataFlow::Node node1, string state1, DataFlow::Node node2, string state2
33+
predicate isAdditionalFlowStep(
34+
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
3535
) {
3636
// create additional flow steps for `XxeFlowStateTransformer`s
3737
state2 = node2.asIndirectExpr().(XxeFlowStateTransformer).transform(state1) and
3838
DataFlow::simpleLocalFlowStep(node1, node2)
3939
}
4040

41-
override predicate isBarrier(DataFlow::Node node, string flowstate) {
41+
predicate isBarrier(DataFlow::Node node, FlowState flowstate) {
4242
// when the flowstate is transformed at a call node, block the original
4343
// flowstate value.
4444
node.asIndirectExpr().(XxeFlowStateTransformer).transform(flowstate) != flowstate
4545
}
4646
}
4747

48-
from XxeConfiguration conf, DataFlow::PathNode source, DataFlow::PathNode sink
49-
where conf.hasFlowPath(source, sink)
48+
module XxeFlow = DataFlow::MakeWithState<XxeConfiguration>;
49+
50+
from XxeFlow::PathNode source, XxeFlow::PathNode sink
51+
where XxeFlow::hasFlowPath(source, sink)
5052
select sink, source, sink,
5153
"This $@ is not configured to prevent an XML external entity (XXE) attack.", source, "XML parser"

‎cpp/ql/src/Security/CWE/CWE-611/Xerces.qll

Lines changed: 33 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -8,38 +8,32 @@ import semmle.code.cpp.valuenumbering.GlobalValueNumbering
88
import semmle.code.cpp.ir.IR
99

1010
/**
11-
* Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
12-
*
13-
* These flow states take the form `Xerces-A-B`, where:
14-
* - A is 1 if `setDisableDefaultEntityResolution` is `true`, 0 otherwise.
15-
* - B is 1 if `setCreateEntityReferenceNodes` is `true`, 0 otherwise.
11+
* A flow state representing the configuration of an `AbstractDOMParser` or
12+
* `SAXParser` object.
1613
*/
17-
predicate encodeXercesFlowState(
18-
string flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
19-
) {
20-
flowstate = "Xerces-0-0" and
21-
disabledDefaultEntityResolution = 0 and
22-
createEntityReferenceNodes = 0
23-
or
24-
flowstate = "Xerces-0-1" and
25-
disabledDefaultEntityResolution = 0 and
26-
createEntityReferenceNodes = 1
27-
or
28-
flowstate = "Xerces-1-0" and
29-
disabledDefaultEntityResolution = 1 and
30-
createEntityReferenceNodes = 0
31-
or
32-
flowstate = "Xerces-1-1" and
33-
disabledDefaultEntityResolution = 1 and
34-
createEntityReferenceNodes = 1
14+
class XercesFlowState extends TXxeFlowState {
15+
int disabledDefaultEntityResolution;
16+
int createEntityReferenceNodes;
17+
18+
XercesFlowState() {
19+
this = TXercesFlowState(disabledDefaultEntityResolution, createEntityReferenceNodes)
20+
}
21+
22+
int getDisabledDefaultEntityResolution() { result = disabledDefaultEntityResolution }
23+
24+
int getCreateEntityReferenceNodes() { result = createEntityReferenceNodes }
25+
26+
string toString() { result = "XercesFlowState" }
3527
}
3628

3729
/**
38-
* A flow state representing the configuration of an `AbstractDOMParser` or
39-
* `SAXParser` object.
30+
* Gets a valid flow state for `AbstractDOMParser` or `SAXParser` flow.
4031
*/
41-
class XercesFlowState extends XxeFlowState {
42-
XercesFlowState() { encodeXercesFlowState(this, _, _) }
32+
predicate encodeXercesFlowState(
33+
XercesFlowState flowstate, int disabledDefaultEntityResolution, int createEntityReferenceNodes
34+
) {
35+
flowstate.getDisabledDefaultEntityResolution() = disabledDefaultEntityResolution and
36+
flowstate.getCreateEntityReferenceNodes() = createEntityReferenceNodes
4337
}
4438

4539
/**
@@ -62,7 +56,7 @@ class XercesDomParserClass extends Class {
6256
class XercesDomParserLibrary extends XmlLibrary {
6357
XercesDomParserLibrary() { this = "XercesDomParserLibrary" }
6458

65-
override predicate configurationSource(DataFlow::Node node, string flowstate) {
59+
override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
6660
// source is the write on `this` of a call to the `XercesDOMParser`
6761
// constructor.
6862
exists(Call call |
@@ -72,7 +66,7 @@ class XercesDomParserLibrary extends XmlLibrary {
7266
)
7367
}
7468

75-
override predicate configurationSink(DataFlow::Node node, string flowstate) {
69+
override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
7670
// sink is the read of the qualifier of a call to `AbstractDOMParser.parse`.
7771
exists(Call call |
7872
call.getTarget().getClassAndName("parse") instanceof AbstractDomParserClass and
@@ -107,7 +101,7 @@ class CreateLSParser extends Function {
107101
class CreateLSParserLibrary extends XmlLibrary {
108102
CreateLSParserLibrary() { this = "CreateLSParserLibrary" }
109103

110-
override predicate configurationSource(DataFlow::Node node, string flowstate) {
104+
override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
111105
// source is the result of a call to `createLSParser`.
112106
exists(Call call |
113107
call.getTarget() instanceof CreateLSParser and
@@ -116,7 +110,7 @@ class CreateLSParserLibrary extends XmlLibrary {
116110
)
117111
}
118112

119-
override predicate configurationSink(DataFlow::Node node, string flowstate) {
113+
override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
120114
// sink is the read of the qualifier of a call to `DOMLSParserClass.parse`.
121115
exists(Call call |
122116
call.getTarget().getClassAndName("parse") instanceof DomLSParserClass and
@@ -147,7 +141,7 @@ class Sax2XmlReader extends Class {
147141
class SaxParserLibrary extends XmlLibrary {
148142
SaxParserLibrary() { this = "SaxParserLibrary" }
149143

150-
override predicate configurationSource(DataFlow::Node node, string flowstate) {
144+
override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
151145
// source is the write on `this` of a call to the `SAXParser`
152146
// constructor.
153147
exists(Call call |
@@ -157,7 +151,7 @@ class SaxParserLibrary extends XmlLibrary {
157151
)
158152
}
159153

160-
override predicate configurationSink(DataFlow::Node node, string flowstate) {
154+
override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
161155
// sink is the read of the qualifier of a call to `SAXParser.parse`.
162156
exists(Call call |
163157
call.getTarget().getClassAndName("parse") instanceof SaxParserClass and
@@ -185,7 +179,7 @@ class CreateXmlReader extends Function {
185179
class Sax2XmlReaderLibrary extends XmlLibrary {
186180
Sax2XmlReaderLibrary() { this = "Sax2XmlReaderLibrary" }
187181

188-
override predicate configurationSource(DataFlow::Node node, string flowstate) {
182+
override predicate configurationSource(DataFlow::Node node, TXxeFlowState flowstate) {
189183
// source is the result of a call to `createXMLReader`.
190184
exists(Call call |
191185
call.getTarget() instanceof CreateXmlReader and
@@ -194,7 +188,7 @@ class Sax2XmlReaderLibrary extends XmlLibrary {
194188
)
195189
}
196190

197-
override predicate configurationSink(DataFlow::Node node, string flowstate) {
191+
override predicate configurationSink(DataFlow::Node node, TXxeFlowState flowstate) {
198192
// sink is the read of the qualifier of a call to `SAX2XMLReader.parse`.
199193
exists(Call call |
200194
call.getTarget().getClassAndName("parse") instanceof Sax2XmlReader and
@@ -227,7 +221,7 @@ class DisableDefaultEntityResolutionTransformer extends XxeFlowStateTransformer
227221
)
228222
}
229223

230-
final override XxeFlowState transform(XxeFlowState flowstate) {
224+
final override TXxeFlowState transform(TXxeFlowState flowstate) {
231225
exists(int createEntityReferenceNodes |
232226
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
233227
(
@@ -258,7 +252,7 @@ class CreateEntityReferenceNodesTransformer extends XxeFlowStateTransformer {
258252
)
259253
}
260254

261-
final override XxeFlowState transform(XxeFlowState flowstate) {
255+
final override TXxeFlowState transform(TXxeFlowState flowstate) {
262256
exists(int disabledDefaultEntityResolution |
263257
encodeXercesFlowState(flowstate, disabledDefaultEntityResolution, _) and
264258
(
@@ -301,7 +295,7 @@ class SetFeatureTransformer extends XxeFlowStateTransformer {
301295
)
302296
}
303297

304-
final override XxeFlowState transform(XxeFlowState flowstate) {
298+
final override TXxeFlowState transform(TXxeFlowState flowstate) {
305299
exists(int createEntityReferenceNodes |
306300
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
307301
(
@@ -359,7 +353,7 @@ class DomConfigurationSetParameterTransformer extends XxeFlowStateTransformer {
359353
)
360354
}
361355

362-
final override XxeFlowState transform(XxeFlowState flowstate) {
356+
final override TXxeFlowState transform(TXxeFlowState flowstate) {
363357
exists(int createEntityReferenceNodes |
364358
encodeXercesFlowState(flowstate, _, createEntityReferenceNodes) and
365359
(

0 commit comments

Comments
 (0)