Skip to content

Commit 76d433d

Browse files
MathiasVPMathiasVP
authored
Merge pull request #9997 from rdmarsh2/rdmarsh2/cpp/product-flow
C++: Experimental product flow library
2 parents 55a10d9 + e37848e commit 76d433d

File tree

9 files changed

+395
-0
lines changed

9 files changed

+395
-0
lines changed

‎cpp/ql/lib/experimental/semmle/code/cpp/dataflow/ProductFlow.qll

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
import semmle.code.cpp.ir.dataflow.DataFlow
2+
import semmle.code.cpp.ir.dataflow.DataFlow2
3+
4+
module ProductFlow {
5+
abstract class Configuration extends string {
6+
bindingset[this]
7+
Configuration() { any() }
8+
9+
/**
10+
* Holds if `(source1, source2)` is a relevant data flow source.
11+
*
12+
* `source1` and `source2` must belong to the same callable.
13+
*/
14+
abstract predicate isSourcePair(DataFlow::Node source1, DataFlow::Node source2);
15+
16+
/**
17+
* Holds if `(sink1, sink2)` is a relevant data flow sink.
18+
*
19+
* `sink1` and `sink2` must belong to the same callable.
20+
*/
21+
abstract predicate isSinkPair(DataFlow::Node sink1, DataFlow::Node sink2);
22+
23+
predicate hasFlowPath(
24+
DataFlow::PathNode source1, DataFlow2::PathNode source2, DataFlow::PathNode sink1,
25+
DataFlow2::PathNode sink2
26+
) {
27+
reachable(this, source1, source2, sink1, sink2)
28+
}
29+
}
30+
31+
private import Internal
32+
33+
module Internal {
34+
class Conf1 extends DataFlow::Configuration {
35+
Conf1() { this = "Conf1" }
36+
37+
override predicate isSource(DataFlow::Node source) {
38+
exists(Configuration conf | conf.isSourcePair(source, _))
39+
}
40+
41+
override predicate isSink(DataFlow::Node sink) {
42+
exists(Configuration conf | conf.isSinkPair(sink, _))
43+
}
44+
}
45+
46+
class Conf2 extends DataFlow2::Configuration {
47+
Conf2() { this = "Conf2" }
48+
49+
override predicate isSource(DataFlow::Node source) {
50+
exists(Configuration conf, DataFlow::Node source1 |
51+
conf.isSourcePair(source1, source) and
52+
any(Conf1 c).hasFlow(source1, _)
53+
)
54+
}
55+
56+
override predicate isSink(DataFlow::Node sink) {
57+
exists(Configuration conf, DataFlow::Node sink1 |
58+
conf.isSinkPair(sink1, sink) and any(Conf1 c).hasFlow(_, sink1)
59+
)
60+
}
61+
}
62+
}
63+
64+
private predicate reachableInterprocEntry(
65+
Configuration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2,
66+
DataFlow::PathNode node1, DataFlow2::PathNode node2
67+
) {
68+
conf.isSourcePair(node1.getNode(), node2.getNode()) and
69+
node1 = source1 and
70+
node2 = source2
71+
or
72+
exists(
73+
DataFlow::PathNode midEntry1, DataFlow2::PathNode midEntry2, DataFlow::PathNode midExit1,
74+
DataFlow2::PathNode midExit2
75+
|
76+
reachableInterprocEntry(conf, source1, source2, midEntry1, midEntry2) and
77+
interprocEdgePair(midExit1, midExit2, node1, node2) and
78+
localPathStep1*(midEntry1, midExit1) and
79+
localPathStep2*(midEntry2, midExit2)
80+
)
81+
}
82+
83+
private predicate localPathStep1(DataFlow::PathNode pred, DataFlow::PathNode succ) {
84+
DataFlow::PathGraph::edges(pred, succ) and
85+
pragma[only_bind_out](pred.getNode().getEnclosingCallable()) =
86+
pragma[only_bind_out](succ.getNode().getEnclosingCallable())
87+
}
88+
89+
private predicate localPathStep2(DataFlow2::PathNode pred, DataFlow2::PathNode succ) {
90+
DataFlow2::PathGraph::edges(pred, succ) and
91+
pragma[only_bind_out](pred.getNode().getEnclosingCallable()) =
92+
pragma[only_bind_out](succ.getNode().getEnclosingCallable())
93+
}
94+
95+
pragma[nomagic]
96+
private predicate interprocEdge1(
97+
Declaration predDecl, Declaration succDecl, DataFlow::PathNode pred1, DataFlow::PathNode succ1
98+
) {
99+
DataFlow::PathGraph::edges(pred1, succ1) and
100+
predDecl != succDecl and
101+
pred1.getNode().getEnclosingCallable() = predDecl and
102+
succ1.getNode().getEnclosingCallable() = succDecl
103+
}
104+
105+
pragma[nomagic]
106+
private predicate interprocEdge2(
107+
Declaration predDecl, Declaration succDecl, DataFlow2::PathNode pred2, DataFlow2::PathNode succ2
108+
) {
109+
DataFlow2::PathGraph::edges(pred2, succ2) and
110+
predDecl != succDecl and
111+
pred2.getNode().getEnclosingCallable() = predDecl and
112+
succ2.getNode().getEnclosingCallable() = succDecl
113+
}
114+
115+
private predicate interprocEdgePair(
116+
DataFlow::PathNode pred1, DataFlow2::PathNode pred2, DataFlow::PathNode succ1,
117+
DataFlow2::PathNode succ2
118+
) {
119+
exists(Declaration predDecl, Declaration succDecl |
120+
interprocEdge1(predDecl, succDecl, pred1, succ1) and
121+
interprocEdge2(predDecl, succDecl, pred2, succ2)
122+
)
123+
}
124+
125+
private predicate reachable(
126+
Configuration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2,
127+
DataFlow::PathNode sink1, DataFlow2::PathNode sink2
128+
) {
129+
exists(DataFlow::PathNode mid1, DataFlow2::PathNode mid2 |
130+
reachableInterprocEntry(conf, source1, source2, mid1, mid2) and
131+
conf.isSinkPair(sink1.getNode(), sink2.getNode()) and
132+
localPathStep1*(mid1, sink1) and
133+
localPathStep2*(mid2, sink2)
134+
)
135+
}
136+
}

‎cpp/ql/src/experimental/Likely Bugs/ArrayAccessProductFlow.ql

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import cpp
2+
import experimental.semmle.code.cpp.dataflow.ProductFlow
3+
import experimental.semmle.code.cpp.semantic.analysis.RangeAnalysis
4+
import experimental.semmle.code.cpp.rangeanalysis.Bound
5+
import experimental.semmle.code.cpp.semantic.SemanticExprSpecific
6+
import semmle.code.cpp.ir.IR
7+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
8+
import semmle.code.cpp.models.interfaces.Allocation
9+
import semmle.code.cpp.ir.IRConfiguration
10+
11+
predicate bounded(Instruction i, Bound b, int delta, boolean upper) {
12+
// TODO: reason
13+
semBounded(getSemanticExpr(i), b, delta, upper, _)
14+
}
15+
16+
class ArraySizeConfiguration extends ProductFlow::Configuration {
17+
ArraySizeConfiguration() { this = "ArraySizeConfiguration" }
18+
19+
override predicate isSourcePair(DataFlow::Node source1, DataFlow::Node source2) {
20+
exists(GVN sizeGvn |
21+
source1.asConvertedExpr().(AllocationExpr).getSizeExpr() = sizeGvn.getAnExpr() and
22+
source2.asConvertedExpr() = sizeGvn.getAnExpr()
23+
)
24+
}
25+
26+
override predicate isSinkPair(DataFlow::Node sink1, DataFlow::Node sink2) {
27+
exists(PointerAddInstruction pai, Instruction index, Bound b, int delta |
28+
pai.getRight() = index and
29+
pai.getLeft() = sink1.asInstruction() and
30+
bounded(index, b, delta, true) and
31+
sink2.asInstruction() = b.getInstruction() and
32+
(
33+
delta = 0 and
34+
exists(DataFlow::Node paiNode, DataFlow::Node derefNode |
35+
DataFlow::localFlow(paiNode, derefNode) and
36+
paiNode.asInstruction() = pai and
37+
derefNode.asOperand() instanceof AddressOperand
38+
)
39+
or
40+
delta >= 1
41+
)
42+
)
43+
}
44+
}
45+
46+
from
47+
ArraySizeConfiguration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2,
48+
DataFlow::PathNode sink1, DataFlow2::PathNode sink2
49+
where conf.hasFlowPath(source1, source2, sink1, sink2)
50+
// TODO: pull delta out and display it
51+
select source1, source2, sink1, sink2

‎cpp/ql/src/experimental/Likely Bugs/OverrunWriteProductFlow.ql

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
import cpp
2+
import experimental.semmle.code.cpp.dataflow.ProductFlow
3+
import semmle.code.cpp.ir.IR
4+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
5+
import semmle.code.cpp.models.interfaces.Allocation
6+
import semmle.code.cpp.models.interfaces.ArrayFunction
7+
8+
class StringSizeConfiguration extends ProductFlow::Configuration {
9+
StringSizeConfiguration() { this = "StringSizeConfiguration" }
10+
11+
override predicate isSourcePair(DataFlow::Node bufSource, DataFlow::Node sizeSource) {
12+
exists(
13+
GVN sizeGvn // TODO: use-use flow instead of GVN
14+
|
15+
bufSource.asConvertedExpr().(AllocationExpr).getSizeExpr() = sizeGvn.getAnExpr() and
16+
sizeSource.asConvertedExpr() = sizeGvn.getAnExpr()
17+
)
18+
}
19+
20+
override predicate isSinkPair(DataFlow::Node bufSink, DataFlow::Node sizeSink) {
21+
exists(CallInstruction c, int bufIndex, int sizeIndex |
22+
c.getStaticCallTarget().(ArrayFunction).hasArrayWithVariableSize(bufIndex, sizeIndex) and
23+
c.getArgument(bufIndex) = bufSink.asInstruction() and
24+
c.getArgument(sizeIndex) = sizeSink.asInstruction()
25+
)
26+
}
27+
}
28+
29+
// we don't actually check correctness yet. Right now the query just finds relevant source/sink pairs.
30+
from
31+
StringSizeConfiguration conf, DataFlow::PathNode source1, DataFlow2::PathNode source2,
32+
DataFlow::PathNode sink1, DataFlow2::PathNode sink2
33+
where conf.hasFlowPath(source1, source2, sink1, sink2)
34+
select source1, source2, sink1, sink2

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| test.cpp:19:19:19:24 | call to malloc | test.cpp:18:17:18:20 | size | test.cpp:26:18:26:23 | string | test.cpp:26:31:26:39 | (size_t)... |
2+
| test.cpp:19:19:19:24 | call to malloc | test.cpp:18:17:18:20 | size | test.cpp:30:18:30:23 | string | test.cpp:30:31:30:39 | (size_t)... |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Likely Bugs/OverrunWriteProductFlow.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/test.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
2+
typedef unsigned long long size_t;
3+
int sprintf(char *s, const char *format, ...);
4+
int snprintf(char *s, size_t n, const char *format, ...);
5+
int scanf(const char *format, ...);
6+
int sscanf(const char *s, const char *format, ...);
7+
char *malloc(size_t size);
8+
char *strncpy(char *dst, const char *src, size_t n);
9+
10+
typedef struct
11+
{
12+
char *string;
13+
int size;
14+
} string_t;
15+
16+
string_t *mk_string_t(int size) {
17+
string_t *str = (string_t *) malloc(sizeof(string_t));
18+
str->size = size;
19+
str->string = malloc(size);
20+
return str;
21+
}
22+
23+
void test1(int size, char *buf) {
24+
string_t *str = mk_string_t(size);
25+
26+
strncpy(str->string, buf, str->size);
27+
}
28+
29+
void strncpy_wrapper(string_t *str, char *buf) {
30+
strncpy(str->string, buf, str->size);
31+
}
32+
33+
void test2(int size, char *buf) {
34+
string_t *str = mk_string_t(size);
35+
strncpy_wrapper(str, buf);
36+
}
37+

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/array-access/ArrayAccessProductFlow.expected

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:4:24:4:27 | size | test.cpp:10:9:10:11 | arr | test.cpp:4:24:4:27 | size |
2+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:4:24:4:27 | size | test.cpp:10:9:10:11 | arr | test.cpp:4:24:4:27 | size |
3+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:4:24:4:27 | size | test.cpp:10:9:10:11 | arr | test.cpp:5:25:5:28 | size |
4+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:4:24:4:27 | size | test.cpp:10:9:10:11 | arr | test.cpp:5:25:5:28 | size |
5+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:4:24:4:27 | size | test.cpp:10:9:10:11 | arr | test.cpp:9:26:9:29 | size |
6+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:5:25:5:28 | size | test.cpp:10:9:10:11 | arr | test.cpp:5:25:5:28 | size |
7+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:5:25:5:28 | size | test.cpp:10:9:10:11 | arr | test.cpp:5:25:5:28 | size |
8+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:5:25:5:28 | size | test.cpp:10:9:10:11 | arr | test.cpp:9:26:9:29 | size |
9+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:9:26:9:29 | size | test.cpp:10:9:10:11 | arr | test.cpp:9:26:9:29 | size |
10+
| test.cpp:4:17:4:22 | call to malloc | test.cpp:9:26:9:29 | size | test.cpp:10:9:10:11 | arr | test.cpp:9:26:9:29 | size |
11+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:55:16:55:19 | size | test.cpp:63:13:63:13 | p | test.cpp:55:5:55:19 | Store |
12+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:55:16:55:19 | size | test.cpp:63:13:63:13 | p | test.cpp:55:16:55:19 | size |
13+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:55:16:55:19 | size | test.cpp:63:13:63:13 | p | test.cpp:55:16:55:19 | size |
14+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:55:16:55:19 | size | test.cpp:63:13:63:13 | p | test.cpp:56:20:56:23 | size |
15+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:56:20:56:23 | size | test.cpp:63:13:63:13 | p | test.cpp:56:20:56:23 | size |
16+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:56:20:56:23 | size | test.cpp:63:13:63:13 | p | test.cpp:56:20:56:23 | size |
17+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:58:29:58:32 | size | test.cpp:63:13:63:13 | p | test.cpp:58:29:58:32 | size |
18+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:58:29:58:32 | size | test.cpp:63:13:63:13 | p | test.cpp:58:29:58:32 | size |
19+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:62:30:62:33 | size | test.cpp:63:13:63:13 | p | test.cpp:62:30:62:33 | size |
20+
| test.cpp:56:13:56:18 | call to malloc | test.cpp:62:30:62:33 | size | test.cpp:63:13:63:13 | p | test.cpp:62:30:62:33 | size |
21+
| test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:83:14:83:14 | p | test.cpp:82:31:82:34 | size |
22+
| test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:93:14:93:14 | p | test.cpp:88:30:88:33 | size |
23+
| test.cpp:70:14:70:19 | call to malloc | test.cpp:69:17:69:20 | size | test.cpp:93:14:93:14 | p | test.cpp:92:31:92:34 | size |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/array-access/ArrayAccessProductFlow.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Likely Bugs/ArrayAccessProductFlow.ql

0 commit comments

Comments
 (0)