Skip to content

Commit 3abf5d1

Browse files
rdmarsh2rdmarsh2
committed
C++: stitch paths in array off-by-one query
1 parent 383b2e1 commit 3abf5d1

File tree

2 files changed

+104
-24
lines changed

2 files changed

+104
-24
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-193/ConstantSizeArrayOffByOne.ql

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import semmle.code.cpp.rangeanalysis.new.internal.semantic.analysis.RangeAnalysi
1414
import semmle.code.cpp.rangeanalysis.new.internal.semantic.SemanticExprSpecific
1515
import semmle.code.cpp.ir.IR
1616
import semmle.code.cpp.ir.dataflow.DataFlow
17-
import PointerArithmeticToDerefFlow::PathGraph
17+
import StitchedPathGraph
1818

1919
pragma[nomagic]
2020
Instruction getABoundIn(SemBound b, IRFunction func) {
@@ -93,11 +93,11 @@ predicate isInvalidPointerDerefSink2(DataFlow::Node sink, Instruction i, string
9393
)
9494
}
9595

96-
predicate isConstantSizeOverflowSource(Field f, PointerAddInstruction pai, int delta) {
97-
exists(int size, int bound, DataFlow::Node source, DataFlow::InstructionNode sink |
98-
FieldAddressToPointerArithmeticFlow::flow(source, sink) and
99-
isFieldAddressSource(f, source) and
100-
pai.getLeft() = sink.asInstruction() and
96+
predicate isConstantSizeOverflowSource(Field f, FieldAddressToPointerArithmeticFlow::PathNode fieldSource, PointerAddInstruction pai, int delta) {
97+
exists(int size, int bound, FieldAddressToPointerArithmeticFlow::PathNode sink |
98+
FieldAddressToPointerArithmeticFlow::flowPath(fieldSource, sink) and
99+
isFieldAddressSource(f, fieldSource.getNode()) and
100+
pai.getLeft() = sink.getNode().(DataFlow::InstructionNode).asInstruction() and
101101
f.getUnspecifiedType().(ArrayType).getArraySize() = size and
102102
semBounded(getSemanticExpr(pai.getRight()), any(SemZeroBound b), bound, true, _) and
103103
delta = bound - size and
@@ -109,22 +109,39 @@ predicate isConstantSizeOverflowSource(Field f, PointerAddInstruction pai, int d
109109

110110
module PointerArithmeticToDerefConfig implements DataFlow::ConfigSig {
111111
predicate isSource(DataFlow::Node source) {
112-
isConstantSizeOverflowSource(_, source.asInstruction(), _)
112+
isConstantSizeOverflowSource(_, _, source.asInstruction(), _)
113113
}
114114

115115
pragma[inline]
116116
predicate isSink(DataFlow::Node sink) { isInvalidPointerDerefSink1(sink, _, _) }
117117
}
118118

119+
module MergedPathGraph = DataFlow::MergePathGraph<PointerArithmeticToDerefFlow::PathNode, FieldAddressToPointerArithmeticFlow::PathNode, PointerArithmeticToDerefFlow::PathGraph, FieldAddressToPointerArithmeticFlow::PathGraph>;
120+
class PathNode = MergedPathGraph::PathNode;
121+
module StitchedPathGraph implements DataFlow::PathGraphSig<PathNode>{
122+
query predicate edges(PathNode a, PathNode b) {
123+
MergedPathGraph::PathGraph::edges(a, b)
124+
or
125+
a.asPathNode2().getNode().(DataFlow::InstructionNode).asInstruction() = b.asPathNode1().getNode().(DataFlow::InstructionNode).asInstruction().(PointerAddInstruction).getLeft()
126+
}
127+
128+
query predicate nodes(PathNode n, string key, string val) {
129+
MergedPathGraph::PathGraph::nodes(n, key, val)
130+
}
131+
132+
query predicate subpaths(PathNode arg, PathNode par, PathNode ret, PathNode out) {
133+
MergedPathGraph::PathGraph::subpaths(arg, par, ret, out)
134+
}
135+
}
119136
module PointerArithmeticToDerefFlow = DataFlow::Global<PointerArithmeticToDerefConfig>;
120137

121138
from
122-
Field f, PointerArithmeticToDerefFlow::PathNode source,
123-
PointerArithmeticToDerefFlow::PathNode sink, Instruction deref, string operation, int delta
139+
Field f, PathNode fieldSource, PathNode paiNode,
140+
PathNode sink, Instruction deref, string operation, int delta
124141
where
125-
PointerArithmeticToDerefFlow::flowPath(source, sink) and
142+
PointerArithmeticToDerefFlow::flowPath(paiNode.asPathNode1(), sink.asPathNode1()) and
126143
isInvalidPointerDerefSink2(sink.getNode(), deref, operation) and
127-
isConstantSizeOverflowSource(f, source.getNode().asInstruction(), delta)
128-
select source, source, sink,
144+
isConstantSizeOverflowSource(f, fieldSource.asPathNode2(), paiNode.getNode().asInstruction(), delta)
145+
select paiNode, fieldSource, sink,
129146
"This pointer arithmetic may have an off-by-" + (delta + 1) +
130147
" error allowing it to overrun $@ at this $@.", f, f.getName(), deref, operation

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-193/constant-size/ConstantSizeArrayOffByOne.expected

Lines changed: 75 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,100 @@
11
edges
2+
| test.cpp:26:10:26:12 | buf | test.cpp:26:5:26:12 | buf |
3+
| test.cpp:30:10:30:12 | buf | test.cpp:30:5:30:12 | buf |
4+
| test.cpp:34:10:34:12 | buf | test.cpp:34:5:34:12 | buf |
5+
| test.cpp:35:5:35:12 | buf | test.cpp:35:5:35:22 | access to array |
6+
| test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:12 | buf |
7+
| test.cpp:36:5:36:12 | buf | test.cpp:36:5:36:24 | access to array |
8+
| test.cpp:36:10:36:12 | buf | test.cpp:36:5:36:12 | buf |
9+
| test.cpp:39:14:39:16 | buf | test.cpp:39:9:39:16 | buf |
10+
| test.cpp:43:9:43:16 | buf | test.cpp:43:9:43:19 | access to array |
11+
| test.cpp:43:14:43:16 | buf | test.cpp:43:9:43:16 | buf |
12+
| test.cpp:48:10:48:12 | buf | test.cpp:48:5:48:12 | buf |
13+
| test.cpp:49:5:49:12 | buf | test.cpp:49:5:49:22 | access to array |
14+
| test.cpp:49:10:49:12 | buf | test.cpp:49:5:49:12 | buf |
15+
| test.cpp:50:5:50:12 | buf | test.cpp:50:5:50:24 | access to array |
16+
| test.cpp:50:10:50:12 | buf | test.cpp:50:5:50:12 | buf |
17+
| test.cpp:53:14:53:16 | buf | test.cpp:53:9:53:16 | buf |
18+
| test.cpp:57:9:57:16 | buf | test.cpp:57:9:57:19 | access to array |
19+
| test.cpp:57:14:57:16 | buf | test.cpp:57:9:57:16 | buf |
20+
| test.cpp:61:9:61:16 | buf | test.cpp:61:9:61:19 | access to array |
21+
| test.cpp:61:14:61:16 | buf | test.cpp:61:9:61:16 | buf |
222
| test.cpp:66:32:66:32 | p | test.cpp:66:32:66:32 | p |
323
| test.cpp:66:32:66:32 | p | test.cpp:67:5:67:6 | * ... |
424
| test.cpp:66:32:66:32 | p | test.cpp:67:6:67:6 | p |
25+
| test.cpp:70:33:70:33 | p | test.cpp:71:5:71:5 | p |
26+
| test.cpp:70:33:70:33 | p | test.cpp:72:5:72:5 | p |
27+
| test.cpp:72:5:72:5 | p | test.cpp:72:5:72:15 | access to array |
28+
| test.cpp:76:32:76:34 | buf | test.cpp:76:27:76:34 | buf |
529
| test.cpp:77:26:77:44 | & ... | test.cpp:66:32:66:32 | p |
630
| test.cpp:77:26:77:44 | & ... | test.cpp:66:32:66:32 | p |
31+
| test.cpp:77:27:77:34 | buf | test.cpp:77:27:77:44 | access to array |
732
| test.cpp:77:27:77:44 | access to array | test.cpp:77:26:77:44 | & ... |
33+
| test.cpp:77:32:77:34 | buf | test.cpp:77:27:77:34 | buf |
34+
| test.cpp:79:27:79:34 | buf | test.cpp:70:33:70:33 | p |
35+
| test.cpp:79:32:79:34 | buf | test.cpp:79:27:79:34 | buf |
836
nodes
37+
| test.cpp:26:5:26:12 | buf | semmle.label | buf |
38+
| test.cpp:26:10:26:12 | buf | semmle.label | buf |
39+
| test.cpp:30:5:30:12 | buf | semmle.label | buf |
40+
| test.cpp:30:10:30:12 | buf | semmle.label | buf |
41+
| test.cpp:34:5:34:12 | buf | semmle.label | buf |
42+
| test.cpp:34:10:34:12 | buf | semmle.label | buf |
43+
| test.cpp:35:5:35:12 | buf | semmle.label | buf |
944
| test.cpp:35:5:35:22 | access to array | semmle.label | access to array |
45+
| test.cpp:35:10:35:12 | buf | semmle.label | buf |
46+
| test.cpp:36:5:36:12 | buf | semmle.label | buf |
1047
| test.cpp:36:5:36:24 | access to array | semmle.label | access to array |
48+
| test.cpp:36:10:36:12 | buf | semmle.label | buf |
49+
| test.cpp:39:9:39:16 | buf | semmle.label | buf |
50+
| test.cpp:39:14:39:16 | buf | semmle.label | buf |
51+
| test.cpp:43:9:43:16 | buf | semmle.label | buf |
1152
| test.cpp:43:9:43:19 | access to array | semmle.label | access to array |
53+
| test.cpp:43:14:43:16 | buf | semmle.label | buf |
54+
| test.cpp:48:5:48:12 | buf | semmle.label | buf |
55+
| test.cpp:48:10:48:12 | buf | semmle.label | buf |
56+
| test.cpp:49:5:49:12 | buf | semmle.label | buf |
1257
| test.cpp:49:5:49:22 | access to array | semmle.label | access to array |
58+
| test.cpp:49:10:49:12 | buf | semmle.label | buf |
59+
| test.cpp:50:5:50:12 | buf | semmle.label | buf |
1360
| test.cpp:50:5:50:24 | access to array | semmle.label | access to array |
61+
| test.cpp:50:10:50:12 | buf | semmle.label | buf |
62+
| test.cpp:53:9:53:16 | buf | semmle.label | buf |
63+
| test.cpp:53:14:53:16 | buf | semmle.label | buf |
64+
| test.cpp:57:9:57:16 | buf | semmle.label | buf |
1465
| test.cpp:57:9:57:19 | access to array | semmle.label | access to array |
66+
| test.cpp:57:14:57:16 | buf | semmle.label | buf |
67+
| test.cpp:61:9:61:16 | buf | semmle.label | buf |
1568
| test.cpp:61:9:61:19 | access to array | semmle.label | access to array |
69+
| test.cpp:61:14:61:16 | buf | semmle.label | buf |
1670
| test.cpp:66:32:66:32 | p | semmle.label | p |
1771
| test.cpp:66:32:66:32 | p | semmle.label | p |
1872
| test.cpp:66:32:66:32 | p | semmle.label | p |
1973
| test.cpp:67:5:67:6 | * ... | semmle.label | * ... |
2074
| test.cpp:67:6:67:6 | p | semmle.label | p |
75+
| test.cpp:70:33:70:33 | p | semmle.label | p |
76+
| test.cpp:71:5:71:5 | p | semmle.label | p |
77+
| test.cpp:72:5:72:5 | p | semmle.label | p |
2178
| test.cpp:72:5:72:15 | access to array | semmle.label | access to array |
79+
| test.cpp:76:27:76:34 | buf | semmle.label | buf |
80+
| test.cpp:76:32:76:34 | buf | semmle.label | buf |
2281
| test.cpp:77:26:77:44 | & ... | semmle.label | & ... |
82+
| test.cpp:77:27:77:34 | buf | semmle.label | buf |
2383
| test.cpp:77:27:77:44 | access to array | semmle.label | access to array |
84+
| test.cpp:77:32:77:34 | buf | semmle.label | buf |
85+
| test.cpp:79:27:79:34 | buf | semmle.label | buf |
86+
| test.cpp:79:32:79:34 | buf | semmle.label | buf |
2487
subpaths
2588
#select
26-
| test.cpp:35:5:35:22 | access to array | test.cpp:35:5:35:22 | access to array | test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write |
27-
| test.cpp:36:5:36:24 | access to array | test.cpp:36:5:36:24 | access to array | test.cpp:36:5:36:24 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:36:5:36:28 | Store: ... = ... | write |
28-
| test.cpp:43:9:43:19 | access to array | test.cpp:43:9:43:19 | access to array | test.cpp:43:9:43:19 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:43:9:43:23 | Store: ... = ... | write |
29-
| test.cpp:49:5:49:22 | access to array | test.cpp:49:5:49:22 | access to array | test.cpp:49:5:49:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:49:5:49:26 | Store: ... = ... | write |
30-
| test.cpp:50:5:50:24 | access to array | test.cpp:50:5:50:24 | access to array | test.cpp:50:5:50:24 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:50:5:50:28 | Store: ... = ... | write |
31-
| test.cpp:57:9:57:19 | access to array | test.cpp:57:9:57:19 | access to array | test.cpp:57:9:57:19 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:57:9:57:23 | Store: ... = ... | write |
32-
| test.cpp:61:9:61:19 | access to array | test.cpp:61:9:61:19 | access to array | test.cpp:61:9:61:19 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:61:9:61:23 | Store: ... = ... | write |
33-
| test.cpp:72:5:72:15 | access to array | test.cpp:72:5:72:15 | access to array | test.cpp:72:5:72:15 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:72:5:72:19 | Store: ... = ... | write |
34-
| test.cpp:77:27:77:44 | access to array | test.cpp:77:27:77:44 | access to array | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
35-
| test.cpp:77:27:77:44 | access to array | test.cpp:77:27:77:44 | access to array | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
36-
| test.cpp:77:27:77:44 | access to array | test.cpp:77:27:77:44 | access to array | test.cpp:67:5:67:6 | * ... | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
37-
| test.cpp:77:27:77:44 | access to array | test.cpp:77:27:77:44 | access to array | test.cpp:67:6:67:6 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
89+
| test.cpp:35:5:35:22 | access to array | test.cpp:35:10:35:12 | buf | test.cpp:35:5:35:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:35:5:35:26 | Store: ... = ... | write |
90+
| test.cpp:36:5:36:24 | access to array | test.cpp:36:10:36:12 | buf | test.cpp:36:5:36:24 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:36:5:36:28 | Store: ... = ... | write |
91+
| test.cpp:43:9:43:19 | access to array | test.cpp:43:14:43:16 | buf | test.cpp:43:9:43:19 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:43:9:43:23 | Store: ... = ... | write |
92+
| test.cpp:49:5:49:22 | access to array | test.cpp:49:10:49:12 | buf | test.cpp:49:5:49:22 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:49:5:49:26 | Store: ... = ... | write |
93+
| test.cpp:50:5:50:24 | access to array | test.cpp:50:10:50:12 | buf | test.cpp:50:5:50:24 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:50:5:50:28 | Store: ... = ... | write |
94+
| test.cpp:57:9:57:19 | access to array | test.cpp:57:14:57:16 | buf | test.cpp:57:9:57:19 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:57:9:57:23 | Store: ... = ... | write |
95+
| test.cpp:61:9:61:19 | access to array | test.cpp:61:14:61:16 | buf | test.cpp:61:9:61:19 | access to array | This pointer arithmetic may have an off-by-2 error allowing it to overrun $@ at this $@. | test.cpp:19:9:19:11 | buf | buf | test.cpp:61:9:61:23 | Store: ... = ... | write |
96+
| test.cpp:72:5:72:15 | access to array | test.cpp:79:32:79:34 | buf | test.cpp:72:5:72:15 | access to array | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:72:5:72:19 | Store: ... = ... | write |
97+
| test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
98+
| test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:66:32:66:32 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
99+
| test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:67:5:67:6 | * ... | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |
100+
| test.cpp:77:27:77:44 | access to array | test.cpp:77:32:77:34 | buf | test.cpp:67:6:67:6 | p | This pointer arithmetic may have an off-by-1 error allowing it to overrun $@ at this $@. | test.cpp:15:9:15:11 | buf | buf | test.cpp:67:5:67:10 | Store: ... = ... | write |

0 commit comments

Comments
 (0)