Skip to content

Commit 94364f7

Browse files
committed
C++: Remove CP between all sinks and all states in 'cpp/iterator-to-expired-container'.
1 parent 0fa5a1f commit 94364f7

File tree

1 file changed

+37
-9
lines changed

1 file changed

+37
-9
lines changed

cpp/ql/src/Security/CWE/CWE-416/IteratorToExpiredContainer.ql

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,31 @@ predicate destroyedToBeginSink(DataFlow::Node sink, FunctionCall fc) {
7070
)
7171
}
7272

73+
/**
74+
* Holds if `node1` is the node corresponding to a qualifier of a destructor
75+
* call and `node2` is a node that is destroyed as a result of `node1` being
76+
* destroyed.
77+
*/
78+
private predicate qualifierToDestroyed(DataFlow::Node node1, DataFlow::Node node2) {
79+
tempToDestructorSink(node1, _) and
80+
node2 = getADestroyedNode(node1)
81+
}
82+
83+
/**
84+
* A configuration to track flow from a destroyed node to a qualifier of
85+
* a `begin` or `end` function call.
86+
*
87+
* This configuration exists to prevent a cartesian product between all sinks and
88+
* all states in `Config::isSink`.
89+
*/
90+
module Config0 implements DataFlow::ConfigSig {
91+
predicate isSource(DataFlow::Node source) { qualifierToDestroyed(_, source) }
92+
93+
predicate isSink(DataFlow::Node sink) { destroyedToBeginSink(sink, _) }
94+
}
95+
96+
module Flow0 = DataFlow::Global<Config0>;
97+
7398
/**
7499
* A configuration to track flow from a temporary variable to the qualifier of
75100
* a destructor call, and subsequently to a qualifier of a call to `begin` or
@@ -79,12 +104,15 @@ module Config implements DataFlow::StateConfigSig {
79104
newtype FlowState =
80105
additional TempToDestructor() or
81106
additional DestroyedToBegin(DataFlow::Node n) {
82-
exists(DataFlow::Node thisOperand |
83-
tempToDestructorSink(thisOperand, _) and
84-
n = getADestroyedNode(thisOperand)
85-
)
107+
any(Flow0::PathNode pn | pn.isSource()).getNode() = n
86108
}
87109

110+
/**
111+
* Holds if `sink` is a qualifier to a call to `begin`, and `mid` is an
112+
* object that is destroyed.
113+
*/
114+
private predicate relevant(DataFlow::Node mid, DataFlow::Node sink) { Flow0::flow(mid, sink) }
115+
88116
predicate isSource(DataFlow::Node source, FlowState state) {
89117
source.asInstruction().(VariableAddressInstruction).getIRVariable() instanceof IRTempVariable and
90118
state = TempToDestructor()
@@ -93,16 +121,16 @@ module Config implements DataFlow::StateConfigSig {
93121
predicate isAdditionalFlowStep(
94122
DataFlow::Node node1, FlowState state1, DataFlow::Node node2, FlowState state2
95123
) {
96-
tempToDestructorSink(node1, _) and
97124
state1 = TempToDestructor() and
98125
state2 = DestroyedToBegin(node2) and
99-
node2 = getADestroyedNode(node1)
126+
qualifierToDestroyed(node1, node2)
100127
}
101128

102129
predicate isSink(DataFlow::Node sink, FlowState state) {
103-
// Note: This is a non-trivial cartesian product!
104-
// Hopefully, both of these sets are quite small in practice
105-
destroyedToBeginSink(sink, _) and state instanceof DestroyedToBegin
130+
exists(DataFlow::Node mid |
131+
relevant(mid, sink) and
132+
state = DestroyedToBegin(mid)
133+
)
106134
}
107135

108136
DataFlow::FlowFeature getAFeature() {

0 commit comments

Comments
 (0)