Skip to content

Commit f22979f

Browse files
authored
Merge pull request #14561 from jketema/rewrite-uncontrolled-process-operation
C++: Rewrite `cpp/uncontrolled-process-operation` to not use `DefaultTaintTracking`
2 parents 803ed20 + 46e6e72 commit f22979f

File tree

3 files changed

+80
-133
lines changed

3 files changed

+80
-133
lines changed

cpp/ql/src/Security/CWE/CWE-114/UncontrolledProcessOperation.ql

Lines changed: 33 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,25 +14,47 @@
1414

1515
import cpp
1616
import semmle.code.cpp.security.Security
17-
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
18-
import TaintedWithPath
17+
import semmle.code.cpp.security.FlowSources
18+
import semmle.code.cpp.ir.dataflow.TaintTracking
19+
import semmle.code.cpp.ir.IR
20+
import Flow::PathGraph
1921

20-
predicate isProcessOperationExplanation(Expr arg, string processOperation) {
22+
predicate isProcessOperationExplanation(DataFlow::Node arg, string processOperation) {
2123
exists(int processOperationArg, FunctionCall call |
2224
isProcessOperationArgument(processOperation, processOperationArg) and
2325
call.getTarget().getName() = processOperation and
24-
call.getArgument(processOperationArg) = arg
26+
call.getArgument(processOperationArg) = [arg.asExpr(), arg.asIndirectExpr()]
2527
)
2628
}
2729

28-
class Configuration extends TaintTrackingConfiguration {
29-
override predicate isSink(Element arg) { isProcessOperationExplanation(arg, _) }
30+
predicate isSource(FlowSource source, string sourceType) {
31+
not source instanceof DataFlow::ExprNode and
32+
sourceType = source.getSourceType()
3033
}
3134

32-
from string processOperation, Expr arg, Expr source, PathNode sourceNode, PathNode sinkNode
35+
module Config implements DataFlow::ConfigSig {
36+
predicate isSource(DataFlow::Node node) { isSource(node, _) }
37+
38+
predicate isSink(DataFlow::Node node) { isProcessOperationExplanation(node, _) }
39+
40+
predicate isBarrier(DataFlow::Node node) {
41+
isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType
42+
or
43+
node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType
44+
}
45+
}
46+
47+
module Flow = TaintTracking::Global<Config>;
48+
49+
from
50+
string processOperation, string sourceType, DataFlow::Node source, DataFlow::Node sink,
51+
Flow::PathNode sourceNode, Flow::PathNode sinkNode
3352
where
34-
isProcessOperationExplanation(arg, processOperation) and
35-
taintedWithPath(source, arg, sourceNode, sinkNode)
36-
select arg, sourceNode, sinkNode,
53+
source = sourceNode.getNode() and
54+
sink = sinkNode.getNode() and
55+
isSource(source, sourceType) and
56+
isProcessOperationExplanation(sink, processOperation) and
57+
Flow::flowPath(sourceNode, sinkNode)
58+
select sink, sourceNode, sinkNode,
3759
"The value of this argument may come from $@ and is being passed to " + processOperation + ".",
38-
source, source.toString()
60+
source, sourceType
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,12 @@
11
edges
2-
| test.cpp:37:73:37:76 | data | test.cpp:43:32:43:35 | data |
3-
| test.cpp:37:73:37:76 | data | test.cpp:43:32:43:35 | data |
4-
| test.cpp:37:73:37:76 | data indirection | test.cpp:43:32:43:35 | data |
5-
| test.cpp:37:73:37:76 | data indirection | test.cpp:43:32:43:35 | data |
6-
| test.cpp:64:30:64:35 | call to getenv | test.cpp:73:24:73:27 | data |
7-
| test.cpp:64:30:64:35 | call to getenv | test.cpp:73:24:73:27 | data |
8-
| test.cpp:64:30:64:35 | call to getenv | test.cpp:73:24:73:27 | data indirection |
9-
| test.cpp:64:30:64:35 | call to getenv | test.cpp:73:24:73:27 | data indirection |
10-
| test.cpp:73:24:73:27 | data | test.cpp:37:73:37:76 | data |
2+
| test.cpp:37:73:37:76 | data indirection | test.cpp:43:32:43:35 | data indirection |
3+
| test.cpp:64:30:64:35 | call to getenv indirection | test.cpp:73:24:73:27 | data indirection |
114
| test.cpp:73:24:73:27 | data indirection | test.cpp:37:73:37:76 | data indirection |
12-
subpaths
135
nodes
14-
| test.cpp:37:73:37:76 | data | semmle.label | data |
156
| test.cpp:37:73:37:76 | data indirection | semmle.label | data indirection |
16-
| test.cpp:43:32:43:35 | data | semmle.label | data |
17-
| test.cpp:43:32:43:35 | data | semmle.label | data |
18-
| test.cpp:64:30:64:35 | call to getenv | semmle.label | call to getenv |
19-
| test.cpp:64:30:64:35 | call to getenv | semmle.label | call to getenv |
20-
| test.cpp:73:24:73:27 | data | semmle.label | data |
7+
| test.cpp:43:32:43:35 | data indirection | semmle.label | data indirection |
8+
| test.cpp:64:30:64:35 | call to getenv indirection | semmle.label | call to getenv indirection |
219
| test.cpp:73:24:73:27 | data indirection | semmle.label | data indirection |
10+
subpaths
2211
#select
23-
| test.cpp:43:32:43:35 | data | test.cpp:64:30:64:35 | call to getenv | test.cpp:43:32:43:35 | data | The value of this argument may come from $@ and is being passed to LoadLibraryA. | test.cpp:64:30:64:35 | call to getenv | call to getenv |
12+
| test.cpp:43:32:43:35 | data indirection | test.cpp:64:30:64:35 | call to getenv indirection | test.cpp:43:32:43:35 | data indirection | The value of this argument may come from $@ and is being passed to LoadLibraryA. | test.cpp:64:30:64:35 | call to getenv indirection | an environment variable |
Original file line numberDiff line numberDiff line change
@@ -1,112 +1,48 @@
11
edges
2-
| test.cpp:24:30:24:36 | command | test.cpp:26:10:26:16 | command |
3-
| test.cpp:24:30:24:36 | command | test.cpp:26:10:26:16 | command |
4-
| test.cpp:29:30:29:36 | command | test.cpp:31:10:31:16 | command |
5-
| test.cpp:29:30:29:36 | command | test.cpp:31:10:31:16 | command |
6-
| test.cpp:42:18:42:23 | call to getenv | test.cpp:24:30:24:36 | command |
7-
| test.cpp:42:18:42:34 | call to getenv | test.cpp:24:30:24:36 | command |
8-
| test.cpp:43:18:43:23 | call to getenv | test.cpp:29:30:29:36 | command |
9-
| test.cpp:43:18:43:34 | call to getenv | test.cpp:29:30:29:36 | command |
10-
| test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer |
11-
| test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer |
12-
| test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer |
13-
| test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer |
14-
| test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data |
15-
| test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data |
16-
| test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data |
17-
| test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data |
18-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
19-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
20-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
21-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
22-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
23-
| test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref |
24-
| test.cpp:56:12:56:17 | buffer | test.cpp:65:10:65:14 | data2 |
25-
| test.cpp:56:12:56:17 | buffer | test.cpp:65:10:65:14 | data2 |
26-
| test.cpp:56:12:56:17 | buffer | test.cpp:65:10:65:14 | data2 |
27-
| test.cpp:56:12:56:17 | buffer | test.cpp:65:10:65:14 | data2 |
28-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer |
29-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer |
30-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data |
31-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data |
32-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref |
33-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref |
34-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref |
35-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 |
36-
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 |
37-
| test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer |
38-
| test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer |
39-
| test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer |
40-
| test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer |
41-
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer |
42-
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer |
43-
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer |
44-
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer |
45-
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer |
46-
| test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer |
47-
| test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | buffer |
48-
| test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | buffer |
49-
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer |
50-
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer |
51-
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer |
52-
| test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer |
53-
| test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | buffer |
54-
| test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | buffer |
55-
| test.cpp:113:8:113:12 | call to fgets | test.cpp:114:9:114:11 | ptr |
56-
| test.cpp:113:8:113:12 | call to fgets | test.cpp:114:9:114:11 | ptr |
57-
| test.cpp:113:8:113:12 | call to fgets | test.cpp:114:9:114:11 | ptr |
58-
| test.cpp:113:8:113:12 | call to fgets | test.cpp:114:9:114:11 | ptr |
59-
subpaths
2+
| test.cpp:24:30:24:36 | command indirection | test.cpp:26:10:26:16 | command indirection |
3+
| test.cpp:29:30:29:36 | command indirection | test.cpp:31:10:31:16 | command indirection |
4+
| test.cpp:42:18:42:34 | call to getenv indirection | test.cpp:24:30:24:36 | command indirection |
5+
| test.cpp:43:18:43:34 | call to getenv indirection | test.cpp:29:30:29:36 | command indirection |
6+
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer indirection |
7+
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data indirection |
8+
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | (reference dereference) indirection |
9+
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref indirection |
10+
| test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 indirection |
11+
| test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer indirection |
12+
| test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | buffer indirection |
13+
| test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | buffer indirection |
14+
| test.cpp:113:8:113:12 | call to fgets indirection | test.cpp:114:9:114:11 | ptr indirection |
6015
nodes
61-
| test.cpp:24:30:24:36 | command | semmle.label | command |
62-
| test.cpp:26:10:26:16 | command | semmle.label | command |
63-
| test.cpp:26:10:26:16 | command | semmle.label | command |
64-
| test.cpp:29:30:29:36 | command | semmle.label | command |
65-
| test.cpp:31:10:31:16 | command | semmle.label | command |
66-
| test.cpp:31:10:31:16 | command | semmle.label | command |
67-
| test.cpp:42:18:42:23 | call to getenv | semmle.label | call to getenv |
68-
| test.cpp:42:18:42:34 | call to getenv | semmle.label | call to getenv |
69-
| test.cpp:43:18:43:23 | call to getenv | semmle.label | call to getenv |
70-
| test.cpp:43:18:43:34 | call to getenv | semmle.label | call to getenv |
71-
| test.cpp:56:12:56:17 | buffer | semmle.label | buffer |
72-
| test.cpp:56:12:56:17 | buffer | semmle.label | buffer |
16+
| test.cpp:24:30:24:36 | command indirection | semmle.label | command indirection |
17+
| test.cpp:26:10:26:16 | command indirection | semmle.label | command indirection |
18+
| test.cpp:29:30:29:36 | command indirection | semmle.label | command indirection |
19+
| test.cpp:31:10:31:16 | command indirection | semmle.label | command indirection |
20+
| test.cpp:42:18:42:34 | call to getenv indirection | semmle.label | call to getenv indirection |
21+
| test.cpp:43:18:43:34 | call to getenv indirection | semmle.label | call to getenv indirection |
7322
| test.cpp:56:12:56:17 | fgets output argument | semmle.label | fgets output argument |
74-
| test.cpp:62:10:62:15 | buffer | semmle.label | buffer |
75-
| test.cpp:62:10:62:15 | buffer | semmle.label | buffer |
76-
| test.cpp:63:10:63:13 | data | semmle.label | data |
77-
| test.cpp:63:10:63:13 | data | semmle.label | data |
78-
| test.cpp:64:10:64:16 | dataref | semmle.label | dataref |
79-
| test.cpp:64:10:64:16 | dataref | semmle.label | dataref |
80-
| test.cpp:64:10:64:16 | dataref | semmle.label | dataref |
81-
| test.cpp:65:10:65:14 | data2 | semmle.label | data2 |
82-
| test.cpp:65:10:65:14 | data2 | semmle.label | data2 |
83-
| test.cpp:76:12:76:17 | buffer | semmle.label | buffer |
84-
| test.cpp:76:12:76:17 | buffer | semmle.label | buffer |
23+
| test.cpp:62:10:62:15 | buffer indirection | semmle.label | buffer indirection |
24+
| test.cpp:63:10:63:13 | data indirection | semmle.label | data indirection |
25+
| test.cpp:64:10:64:16 | (reference dereference) indirection | semmle.label | (reference dereference) indirection |
26+
| test.cpp:64:10:64:16 | dataref indirection | semmle.label | dataref indirection |
27+
| test.cpp:65:10:65:14 | data2 indirection | semmle.label | data2 indirection |
8528
| test.cpp:76:12:76:17 | fgets output argument | semmle.label | fgets output argument |
86-
| test.cpp:78:10:78:15 | buffer | semmle.label | buffer |
87-
| test.cpp:78:10:78:15 | buffer | semmle.label | buffer |
88-
| test.cpp:98:17:98:22 | buffer | semmle.label | buffer |
89-
| test.cpp:98:17:98:22 | buffer | semmle.label | buffer |
29+
| test.cpp:78:10:78:15 | buffer indirection | semmle.label | buffer indirection |
9030
| test.cpp:98:17:98:22 | recv output argument | semmle.label | recv output argument |
91-
| test.cpp:99:15:99:20 | buffer | semmle.label | buffer |
92-
| test.cpp:99:15:99:20 | buffer | semmle.label | buffer |
93-
| test.cpp:106:17:106:22 | buffer | semmle.label | buffer |
94-
| test.cpp:106:17:106:22 | buffer | semmle.label | buffer |
31+
| test.cpp:99:15:99:20 | buffer indirection | semmle.label | buffer indirection |
9532
| test.cpp:106:17:106:22 | recv output argument | semmle.label | recv output argument |
96-
| test.cpp:107:15:107:20 | buffer | semmle.label | buffer |
97-
| test.cpp:107:15:107:20 | buffer | semmle.label | buffer |
98-
| test.cpp:113:8:113:12 | call to fgets | semmle.label | call to fgets |
99-
| test.cpp:113:8:113:12 | call to fgets | semmle.label | call to fgets |
100-
| test.cpp:114:9:114:11 | ptr | semmle.label | ptr |
101-
| test.cpp:114:9:114:11 | ptr | semmle.label | ptr |
33+
| test.cpp:107:15:107:20 | buffer indirection | semmle.label | buffer indirection |
34+
| test.cpp:113:8:113:12 | call to fgets indirection | semmle.label | call to fgets indirection |
35+
| test.cpp:114:9:114:11 | ptr indirection | semmle.label | ptr indirection |
36+
subpaths
10237
#select
103-
| test.cpp:26:10:26:16 | command | test.cpp:42:18:42:23 | call to getenv | test.cpp:26:10:26:16 | command | The value of this argument may come from $@ and is being passed to system. | test.cpp:42:18:42:23 | call to getenv | call to getenv |
104-
| test.cpp:31:10:31:16 | command | test.cpp:43:18:43:23 | call to getenv | test.cpp:31:10:31:16 | command | The value of this argument may come from $@ and is being passed to system. | test.cpp:43:18:43:23 | call to getenv | call to getenv |
105-
| test.cpp:62:10:62:15 | buffer | test.cpp:56:12:56:17 | buffer | test.cpp:62:10:62:15 | buffer | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | buffer | buffer |
106-
| test.cpp:63:10:63:13 | data | test.cpp:56:12:56:17 | buffer | test.cpp:63:10:63:13 | data | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | buffer | buffer |
107-
| test.cpp:64:10:64:16 | dataref | test.cpp:56:12:56:17 | buffer | test.cpp:64:10:64:16 | dataref | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | buffer | buffer |
108-
| test.cpp:65:10:65:14 | data2 | test.cpp:56:12:56:17 | buffer | test.cpp:65:10:65:14 | data2 | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | buffer | buffer |
109-
| test.cpp:78:10:78:15 | buffer | test.cpp:76:12:76:17 | buffer | test.cpp:78:10:78:15 | buffer | The value of this argument may come from $@ and is being passed to system. | test.cpp:76:12:76:17 | buffer | buffer |
110-
| test.cpp:99:15:99:20 | buffer | test.cpp:98:17:98:22 | buffer | test.cpp:99:15:99:20 | buffer | The value of this argument may come from $@ and is being passed to LoadLibrary. | test.cpp:98:17:98:22 | buffer | buffer |
111-
| test.cpp:107:15:107:20 | buffer | test.cpp:106:17:106:22 | buffer | test.cpp:107:15:107:20 | buffer | The value of this argument may come from $@ and is being passed to LoadLibrary. | test.cpp:106:17:106:22 | buffer | buffer |
112-
| test.cpp:114:9:114:11 | ptr | test.cpp:113:8:113:12 | call to fgets | test.cpp:114:9:114:11 | ptr | The value of this argument may come from $@ and is being passed to system. | test.cpp:113:8:113:12 | call to fgets | call to fgets |
38+
| test.cpp:26:10:26:16 | command indirection | test.cpp:42:18:42:34 | call to getenv indirection | test.cpp:26:10:26:16 | command indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:42:18:42:34 | call to getenv indirection | an environment variable |
39+
| test.cpp:31:10:31:16 | command indirection | test.cpp:43:18:43:34 | call to getenv indirection | test.cpp:31:10:31:16 | command indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:43:18:43:34 | call to getenv indirection | an environment variable |
40+
| test.cpp:62:10:62:15 | buffer indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:62:10:62:15 | buffer indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
41+
| test.cpp:63:10:63:13 | data indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:63:10:63:13 | data indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
42+
| test.cpp:64:10:64:16 | (reference dereference) indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | (reference dereference) indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
43+
| test.cpp:64:10:64:16 | dataref indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:64:10:64:16 | dataref indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
44+
| test.cpp:65:10:65:14 | data2 indirection | test.cpp:56:12:56:17 | fgets output argument | test.cpp:65:10:65:14 | data2 indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:56:12:56:17 | fgets output argument | string read by fgets |
45+
| test.cpp:78:10:78:15 | buffer indirection | test.cpp:76:12:76:17 | fgets output argument | test.cpp:78:10:78:15 | buffer indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:76:12:76:17 | fgets output argument | string read by fgets |
46+
| test.cpp:99:15:99:20 | buffer indirection | test.cpp:98:17:98:22 | recv output argument | test.cpp:99:15:99:20 | buffer indirection | The value of this argument may come from $@ and is being passed to LoadLibrary. | test.cpp:98:17:98:22 | recv output argument | buffer read by recv |
47+
| test.cpp:107:15:107:20 | buffer indirection | test.cpp:106:17:106:22 | recv output argument | test.cpp:107:15:107:20 | buffer indirection | The value of this argument may come from $@ and is being passed to LoadLibrary. | test.cpp:106:17:106:22 | recv output argument | buffer read by recv |
48+
| test.cpp:114:9:114:11 | ptr indirection | test.cpp:113:8:113:12 | call to fgets indirection | test.cpp:114:9:114:11 | ptr indirection | The value of this argument may come from $@ and is being passed to system. | test.cpp:113:8:113:12 | call to fgets indirection | string read by fgets |

0 commit comments

Comments
 (0)