Skip to content

Commit e68bb53

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#12435 from jketema/more-config
C++: Convert a number of data flow based queries to use `ConfigSig`
2 parents c84d88f + 5391b13 commit e68bb53

22 files changed

+198
-169
lines changed

‎cpp/ql/src/Critical/OverflowDestination.ql

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import cpp
1717
import semmle.code.cpp.ir.dataflow.TaintTracking
1818
import semmle.code.cpp.controlflow.IRGuards
1919
import semmle.code.cpp.security.FlowSources
20-
import DataFlow::PathGraph
20+
import OverflowDestination::PathGraph
2121

2222
/**
2323
* Holds if `fc` is a call to a copy operation where the size argument contains
@@ -66,14 +66,12 @@ predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Va
6666
any(IRGuardCondition guard).ensuresEq(access, _, _, node.asInstruction().getBlock(), true)
6767
}
6868

69-
class OverflowDestinationConfig extends TaintTracking::Configuration {
70-
OverflowDestinationConfig() { this = "OverflowDestinationConfig" }
69+
module OverflowDestinationConfig implements DataFlow::ConfigSig {
70+
predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
7171

72-
override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
72+
predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asIndirectConvertedExpr()) }
7373

74-
override predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asIndirectConvertedExpr()) }
75-
76-
override predicate isSanitizer(DataFlow::Node node) {
74+
predicate isBarrier(DataFlow::Node node) {
7775
exists(Variable checkedVar |
7876
readsVariable(node.asInstruction(), checkedVar) and
7977
hasUpperBoundsCheck(checkedVar)
@@ -86,11 +84,11 @@ class OverflowDestinationConfig extends TaintTracking::Configuration {
8684
}
8785
}
8886

89-
from
90-
FunctionCall fc, OverflowDestinationConfig conf, DataFlow::PathNode source,
91-
DataFlow::PathNode sink
87+
module OverflowDestination = TaintTracking::Make<OverflowDestinationConfig>;
88+
89+
from FunctionCall fc, OverflowDestination::PathNode source, OverflowDestination::PathNode sink
9290
where
93-
conf.hasFlowPath(source, sink) and
91+
OverflowDestination::hasFlowPath(source, sink) and
9492
sourceSized(fc, sink.getNode().asIndirectConvertedExpr())
9593
select fc, source, sink,
9694
"To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."

‎cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@
1111
import cpp
1212
import NtohlArrayNoBound
1313

14-
from NetworkToBufferSizeConfiguration bufConfig, DataFlow::Node source, DataFlow::Node sink
15-
where bufConfig.hasFlow(source, sink)
14+
from DataFlow::Node source, DataFlow::Node sink
15+
where NetworkToBufferSizeFlow::hasFlow(source, sink)
1616
select sink, "Unchecked use of data from network function $@.", source, source.toString()

‎cpp/ql/src/Likely Bugs/Memory Management/NtohlArrayNoBound.qll

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class NetworkFunctionCall extends FunctionCall {
129129
NetworkFunctionCall() { this.getTarget().hasName(["ntohd", "ntohf", "ntohl", "ntohll", "ntohs"]) }
130130
}
131131

132-
class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {
132+
deprecated class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {
133133
NetworkToBufferSizeConfiguration() { this = "NetworkToBufferSizeConfiguration" }
134134

135135
override predicate isSource(DataFlow::Node node) { node.asExpr() instanceof NetworkFunctionCall }
@@ -146,3 +146,19 @@ class NetworkToBufferSizeConfiguration extends DataFlow::Configuration {
146146
)
147147
}
148148
}
149+
150+
private module NetworkToBufferSizeConfiguration implements DataFlow::ConfigSig {
151+
predicate isSource(DataFlow::Node node) { node.asExpr() instanceof NetworkFunctionCall }
152+
153+
predicate isSink(DataFlow::Node node) { node.asExpr() = any(BufferAccess ba).getAccessedLength() }
154+
155+
predicate isBarrier(DataFlow::Node node) {
156+
exists(GuardCondition gc, GVN gvn |
157+
gc.getAChild*() = gvn.getAnExpr() and
158+
globalValueNumber(node.asExpr()) = gvn and
159+
gc.controls(node.asExpr().getBasicBlock(), _)
160+
)
161+
}
162+
}
163+
164+
module NetworkToBufferSizeFlow = DataFlow::Make<NetworkToBufferSizeConfiguration>;

‎cpp/ql/src/Security/CWE/CWE-020/ExternalAPIs.qll

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ import ExternalAPIsSpecific
1010

1111
/** A node representing untrusted data being passed to an external API. */
1212
class UntrustedExternalApiDataNode extends ExternalApiDataNode {
13-
UntrustedExternalApiDataNode() { any(UntrustedDataToExternalApiConfig c).hasFlow(_, this) }
13+
UntrustedExternalApiDataNode() { UntrustedDataToExternalApiFlow::hasFlow(_, this) }
1414

1515
/** Gets a source of untrusted data which is passed to this external API data node. */
16-
DataFlow::Node getAnUntrustedSource() {
17-
any(UntrustedDataToExternalApiConfig c).hasFlow(result, this)
18-
}
16+
DataFlow::Node getAnUntrustedSource() { UntrustedDataToExternalApiFlow::hasFlow(result, this) }
1917
}
2018

2119
/** DEPRECATED: Alias for UntrustedExternalApiDataNode */

‎cpp/ql/src/Security/CWE/CWE-020/ExternalAPIsSpecific.qll

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ExternalApiDataNode extends DataFlow::Node {
4545
deprecated class ExternalAPIDataNode = ExternalApiDataNode;
4646

4747
/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */
48-
class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
48+
deprecated class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
4949
UntrustedDataToExternalApiConfig() { this = "UntrustedDataToExternalAPIConfig" }
5050

5151
override predicate isSource(DataFlow::Node source) {
@@ -60,3 +60,17 @@ class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
6060

6161
/** DEPRECATED: Alias for UntrustedDataToExternalApiConfig */
6262
deprecated class UntrustedDataToExternalAPIConfig = UntrustedDataToExternalApiConfig;
63+
64+
/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */
65+
private module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig {
66+
predicate isSource(DataFlow::Node source) {
67+
exists(RemoteFlowSourceFunction remoteFlow |
68+
remoteFlow = source.asExpr().(Call).getTarget() and
69+
remoteFlow.hasRemoteFlowSource(_, _)
70+
)
71+
}
72+
73+
predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode }
74+
}
75+
76+
module UntrustedDataToExternalApiFlow = TaintTracking::Make<UntrustedDataToExternalApiConfig>;

‎cpp/ql/src/Security/CWE/CWE-020/IRUntrustedDataToExternalAPI.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import cpp
1313
import semmle.code.cpp.ir.dataflow.TaintTracking
1414
import ir.ExternalAPIs
1515
import semmle.code.cpp.security.FlowSources
16-
import DataFlow::PathGraph
16+
import UntrustedDataToExternalApiFlow::PathGraph
1717

18-
from UntrustedDataToExternalApiConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
19-
where config.hasFlowPath(source, sink)
18+
from UntrustedDataToExternalApiFlow::PathNode source, UntrustedDataToExternalApiFlow::PathNode sink
19+
where UntrustedDataToExternalApiFlow::hasFlowPath(source, sink)
2020
select sink, source, sink,
2121
"Call to " + sink.getNode().(ExternalApiDataNode).getExternalFunction().toString() +
2222
" with untrusted data from $@.", source, source.getNode().(RemoteFlowSource).getSourceType()

‎cpp/ql/src/Security/CWE/CWE-020/UntrustedDataToExternalAPI.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
import cpp
1313
import semmle.code.cpp.ir.dataflow.TaintTracking
1414
import ExternalAPIs
15-
import DataFlow::PathGraph
15+
import UntrustedDataToExternalApiFlow::PathGraph
1616

17-
from UntrustedDataToExternalApiConfig config, DataFlow::PathNode source, DataFlow::PathNode sink
18-
where config.hasFlowPath(source, sink)
17+
from UntrustedDataToExternalApiFlow::PathNode source, UntrustedDataToExternalApiFlow::PathNode sink
18+
where UntrustedDataToExternalApiFlow::hasFlowPath(source, sink)
1919
select sink, source, sink,
2020
"Call to " + sink.getNode().(ExternalApiDataNode).getExternalFunction().toString() +
2121
" with untrusted data from $@.", source, source.toString()

‎cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIs.qll

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ import ExternalAPIsSpecific
1010

1111
/** A node representing untrusted data being passed to an external API. */
1212
class UntrustedExternalApiDataNode extends ExternalApiDataNode {
13-
UntrustedExternalApiDataNode() { any(UntrustedDataToExternalApiConfig c).hasFlow(_, this) }
13+
UntrustedExternalApiDataNode() { UntrustedDataToExternalApiFlow::hasFlow(_, this) }
1414

1515
/** Gets a source of untrusted data which is passed to this external API data node. */
16-
DataFlow::Node getAnUntrustedSource() {
17-
any(UntrustedDataToExternalApiConfig c).hasFlow(result, this)
18-
}
16+
DataFlow::Node getAnUntrustedSource() { UntrustedDataToExternalApiFlow::hasFlow(result, this) }
1917
}
2018

2119
/** DEPRECATED: Alias for UntrustedExternalApiDataNode */

‎cpp/ql/src/Security/CWE/CWE-020/ir/ExternalAPIsSpecific.qll

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ExternalApiDataNode extends DataFlow::Node {
4545
deprecated class ExternalAPIDataNode = ExternalApiDataNode;
4646

4747
/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */
48-
class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
48+
deprecated class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
4949
UntrustedDataToExternalApiConfig() { this = "UntrustedDataToExternalAPIConfigIR" }
5050

5151
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
@@ -55,3 +55,12 @@ class UntrustedDataToExternalApiConfig extends TaintTracking::Configuration {
5555

5656
/** DEPRECATED: Alias for UntrustedDataToExternalApiConfig */
5757
deprecated class UntrustedDataToExternalAPIConfig = UntrustedDataToExternalApiConfig;
58+
59+
/** A configuration for tracking flow from `RemoteFlowSource`s to `ExternalApiDataNode`s. */
60+
private module UntrustedDataToExternalApiConfig implements DataFlow::ConfigSig {
61+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
62+
63+
predicate isSink(DataFlow::Node sink) { sink instanceof ExternalApiDataNode }
64+
}
65+
66+
module UntrustedDataToExternalApiFlow = TaintTracking::Make<UntrustedDataToExternalApiConfig>;

‎cpp/ql/src/Security/CWE/CWE-129/ImproperArrayIndexValidation.ql

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import semmle.code.cpp.controlflow.IRGuards
1717
import semmle.code.cpp.security.FlowSources
1818
import semmle.code.cpp.ir.dataflow.TaintTracking
1919
import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
20-
import DataFlow::PathGraph
20+
import ImproperArrayIndexValidation::PathGraph
2121
import semmle.code.cpp.security.Security
2222

2323
predicate hasUpperBound(VariableAccess offsetExpr) {
@@ -65,12 +65,10 @@ predicate predictableInstruction(Instruction instr) {
6565
predictableInstruction(instr.(UnaryInstruction).getUnary())
6666
}
6767

68-
class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
69-
ImproperArrayIndexValidationConfig() { this = "ImproperArrayIndexValidationConfig" }
68+
module ImproperArrayIndexValidationConfig implements DataFlow::ConfigSig {
69+
predicate isSource(DataFlow::Node source) { isFlowSource(source, _) }
7070

71-
override predicate isSource(DataFlow::Node source) { isFlowSource(source, _) }
72-
73-
override predicate isSanitizer(DataFlow::Node node) {
71+
predicate isBarrier(DataFlow::Node node) {
7472
hasUpperBound(node.asExpr())
7573
or
7674
// These barriers are ported from `DefaultTaintTracking` because this query is quite noisy
@@ -107,7 +105,7 @@ class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
107105
)
108106
}
109107

110-
override predicate isSink(DataFlow::Node sink) {
108+
predicate isSink(DataFlow::Node sink) {
111109
exists(ArrayExpr arrayExpr, VariableAccess offsetExpr |
112110
offsetExpr = arrayExpr.getArrayOffset() and
113111
sink.asExpr() = offsetExpr and
@@ -116,11 +114,13 @@ class ImproperArrayIndexValidationConfig extends TaintTracking::Configuration {
116114
}
117115
}
118116

117+
module ImproperArrayIndexValidation = TaintTracking::Make<ImproperArrayIndexValidationConfig>;
118+
119119
from
120-
ImproperArrayIndexValidationConfig conf, DataFlow::PathNode source, DataFlow::PathNode sink,
120+
ImproperArrayIndexValidation::PathNode source, ImproperArrayIndexValidation::PathNode sink,
121121
string sourceType
122122
where
123-
conf.hasFlowPath(source, sink) and
123+
ImproperArrayIndexValidation::hasFlowPath(source, sink) and
124124
isFlowSource(source.getNode(), sourceType)
125125
select sink.getNode(), source, sink,
126126
"An array indexing expression depends on $@ that might be outside the bounds of the array.",

0 commit comments

Comments
 (0)