Skip to content

Commit 5031b73

Browse files
MathiasVPMathiasVP
committed
C++: Add barrier to cpp/uncontrolled-allocation-size that blocks flow when overflow isn't possible.
1 parent 9b0c24a commit 5031b73

File tree

3 files changed

+19
-33
lines changed

3 files changed

+19
-33
lines changed

‎cpp/ql/src/Security/CWE/CWE-190/TaintedAllocationSize.ql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313

1414
import cpp
15+
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
1516
import semmle.code.cpp.security.TaintTracking
1617
import TaintedWithPath
1718

@@ -27,6 +28,21 @@ predicate allocSink(Expr alloc, Expr tainted) {
2728

2829
class TaintedAllocationSizeConfiguration extends TaintTrackingConfiguration {
2930
override predicate isSink(Element tainted) { allocSink(_, tainted) }
31+
32+
override predicate isBarrier(Expr e) {
33+
super.isBarrier(e)
34+
or
35+
// There can be two separate reasons for `convertedExprMightOverflow` not holding:
36+
// 1. `e` really cannot overflow.
37+
// 2. `e` isn't analyzable.
38+
// If we didn't rule out case 2 we would place barriers on anything that isn't analyzable.
39+
(
40+
e instanceof UnaryArithmeticOperation or
41+
e instanceof BinaryArithmeticOperation or
42+
e instanceof AssignArithmeticOperation
43+
) and
44+
not convertedExprMightOverflow(e)
45+
}
3046
}
3147

3248
predicate taintedAllocSize(

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/TaintedAllocationSize.expected

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,6 @@ edges
5353
| test.cpp:132:19:132:24 | call to getenv | test.cpp:134:10:134:27 | ... * ... |
5454
| test.cpp:132:19:132:32 | (const char *)... | test.cpp:134:10:134:27 | ... * ... |
5555
| test.cpp:132:19:132:32 | (const char *)... | test.cpp:134:10:134:27 | ... * ... |
56-
| test.cpp:138:19:138:24 | call to getenv | test.cpp:142:11:142:28 | ... * ... |
57-
| test.cpp:138:19:138:24 | call to getenv | test.cpp:142:11:142:28 | ... * ... |
58-
| test.cpp:138:19:138:32 | (const char *)... | test.cpp:142:11:142:28 | ... * ... |
59-
| test.cpp:138:19:138:32 | (const char *)... | test.cpp:142:11:142:28 | ... * ... |
6056
| test.cpp:201:9:201:42 | Store | test.cpp:231:9:231:24 | call to get_tainted_size |
6157
| test.cpp:201:9:201:42 | Store | test.cpp:231:9:231:24 | call to get_tainted_size |
6258
| test.cpp:201:14:201:19 | call to getenv | test.cpp:201:9:201:42 | Store |
@@ -91,14 +87,6 @@ edges
9187
| test.cpp:295:18:295:21 | Chi | test.cpp:298:10:298:27 | ... * ... |
9288
| test.cpp:295:18:295:21 | Chi | test.cpp:298:10:298:27 | ... * ... |
9389
| test.cpp:295:18:295:21 | get_size output argument [array content] | test.cpp:295:18:295:21 | Chi |
94-
| test.cpp:301:19:301:24 | call to getenv | test.cpp:305:11:305:28 | ... * ... |
95-
| test.cpp:301:19:301:24 | call to getenv | test.cpp:305:11:305:28 | ... * ... |
96-
| test.cpp:301:19:301:32 | (const char *)... | test.cpp:305:11:305:28 | ... * ... |
97-
| test.cpp:301:19:301:32 | (const char *)... | test.cpp:305:11:305:28 | ... * ... |
98-
| test.cpp:309:19:309:24 | call to getenv | test.cpp:314:10:314:27 | ... * ... |
99-
| test.cpp:309:19:309:24 | call to getenv | test.cpp:314:10:314:27 | ... * ... |
100-
| test.cpp:309:19:309:32 | (const char *)... | test.cpp:314:10:314:27 | ... * ... |
101-
| test.cpp:309:19:309:32 | (const char *)... | test.cpp:314:10:314:27 | ... * ... |
10290
nodes
10391
| test.cpp:39:21:39:24 | argv | semmle.label | argv |
10492
| test.cpp:39:21:39:24 | argv | semmle.label | argv |
@@ -149,11 +137,6 @@ nodes
149137
| test.cpp:134:10:134:27 | ... * ... | semmle.label | ... * ... |
150138
| test.cpp:134:10:134:27 | ... * ... | semmle.label | ... * ... |
151139
| test.cpp:134:10:134:27 | ... * ... | semmle.label | ... * ... |
152-
| test.cpp:138:19:138:24 | call to getenv | semmle.label | call to getenv |
153-
| test.cpp:138:19:138:32 | (const char *)... | semmle.label | (const char *)... |
154-
| test.cpp:142:11:142:28 | ... * ... | semmle.label | ... * ... |
155-
| test.cpp:142:11:142:28 | ... * ... | semmle.label | ... * ... |
156-
| test.cpp:142:11:142:28 | ... * ... | semmle.label | ... * ... |
157140
| test.cpp:201:9:201:42 | Store | semmle.label | Store |
158141
| test.cpp:201:14:201:19 | call to getenv | semmle.label | call to getenv |
159142
| test.cpp:201:14:201:27 | (const char *)... | semmle.label | (const char *)... |
@@ -196,16 +179,6 @@ nodes
196179
| test.cpp:298:10:298:27 | ... * ... | semmle.label | ... * ... |
197180
| test.cpp:298:10:298:27 | ... * ... | semmle.label | ... * ... |
198181
| test.cpp:298:10:298:27 | ... * ... | semmle.label | ... * ... |
199-
| test.cpp:301:19:301:24 | call to getenv | semmle.label | call to getenv |
200-
| test.cpp:301:19:301:32 | (const char *)... | semmle.label | (const char *)... |
201-
| test.cpp:305:11:305:28 | ... * ... | semmle.label | ... * ... |
202-
| test.cpp:305:11:305:28 | ... * ... | semmle.label | ... * ... |
203-
| test.cpp:305:11:305:28 | ... * ... | semmle.label | ... * ... |
204-
| test.cpp:309:19:309:24 | call to getenv | semmle.label | call to getenv |
205-
| test.cpp:309:19:309:32 | (const char *)... | semmle.label | (const char *)... |
206-
| test.cpp:314:10:314:27 | ... * ... | semmle.label | ... * ... |
207-
| test.cpp:314:10:314:27 | ... * ... | semmle.label | ... * ... |
208-
| test.cpp:314:10:314:27 | ... * ... | semmle.label | ... * ... |
209182
#select
210183
| test.cpp:42:31:42:36 | call to malloc | test.cpp:39:21:39:24 | argv | test.cpp:42:38:42:44 | tainted | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
211184
| test.cpp:43:31:43:36 | call to malloc | test.cpp:39:21:39:24 | argv | test.cpp:43:38:43:63 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:39:21:39:24 | argv | user input (argv) |
@@ -216,13 +189,10 @@ nodes
216189
| test.cpp:79:9:79:29 | new[] | test.cpp:97:18:97:23 | buffer | test.cpp:79:18:79:28 | ... - ... | This allocation size is derived from $@ and might overflow | test.cpp:97:18:97:23 | buffer | user input (fread) |
217190
| test.cpp:127:17:127:22 | call to malloc | test.cpp:123:18:123:23 | call to getenv | test.cpp:127:24:127:41 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:123:18:123:23 | call to getenv | user input (getenv) |
218191
| test.cpp:134:3:134:8 | call to malloc | test.cpp:132:19:132:24 | call to getenv | test.cpp:134:10:134:27 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:132:19:132:24 | call to getenv | user input (getenv) |
219-
| test.cpp:142:4:142:9 | call to malloc | test.cpp:138:19:138:24 | call to getenv | test.cpp:142:11:142:28 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:138:19:138:24 | call to getenv | user input (getenv) |
220192
| test.cpp:215:14:215:19 | call to malloc | test.cpp:227:24:227:29 | call to getenv | test.cpp:215:21:215:21 | s | This allocation size is derived from $@ and might overflow | test.cpp:227:24:227:29 | call to getenv | user input (getenv) |
221193
| test.cpp:221:14:221:19 | call to malloc | test.cpp:227:24:227:29 | call to getenv | test.cpp:221:21:221:21 | s | This allocation size is derived from $@ and might overflow | test.cpp:227:24:227:29 | call to getenv | user input (getenv) |
222194
| test.cpp:229:2:229:7 | call to malloc | test.cpp:227:24:227:29 | call to getenv | test.cpp:229:9:229:18 | local_size | This allocation size is derived from $@ and might overflow | test.cpp:227:24:227:29 | call to getenv | user input (getenv) |
223195
| test.cpp:231:2:231:7 | call to malloc | test.cpp:201:14:201:19 | call to getenv | test.cpp:231:9:231:24 | call to get_tainted_size | This allocation size is derived from $@ and might overflow | test.cpp:201:14:201:19 | call to getenv | user input (getenv) |
224196
| test.cpp:253:4:253:9 | call to malloc | test.cpp:249:20:249:25 | call to getenv | test.cpp:253:11:253:29 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:249:20:249:25 | call to getenv | user input (getenv) |
225197
| test.cpp:281:4:281:9 | call to malloc | test.cpp:241:18:241:23 | call to getenv | test.cpp:281:11:281:28 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:241:18:241:23 | call to getenv | user input (getenv) |
226198
| test.cpp:298:3:298:8 | call to malloc | test.cpp:241:18:241:23 | call to getenv | test.cpp:298:10:298:27 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:241:18:241:23 | call to getenv | user input (getenv) |
227-
| test.cpp:305:4:305:9 | call to malloc | test.cpp:301:19:301:24 | call to getenv | test.cpp:305:11:305:28 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:301:19:301:24 | call to getenv | user input (getenv) |
228-
| test.cpp:314:3:314:8 | call to malloc | test.cpp:309:19:309:24 | call to getenv | test.cpp:314:10:314:27 | ... * ... | This allocation size is derived from $@ and might overflow | test.cpp:309:19:309:24 | call to getenv | user input (getenv) |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/TaintedAllocationSize/test.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ void more_bounded_tests() {
139139

140140
if (size > 0)
141141
{
142-
malloc(size * sizeof(int)); // BAD
142+
malloc(size * sizeof(int)); // GOOD (overflow not possible)
143143
}
144144
}
145145

@@ -302,7 +302,7 @@ void equality_cases() {
302302

303303
if ((size == 50) || (size == 100))
304304
{
305-
malloc(size * sizeof(int)); // GOOD [FALSE POSITIVE]
305+
malloc(size * sizeof(int)); // GOOD
306306
}
307307
}
308308
{
@@ -311,6 +311,6 @@ void equality_cases() {
311311
if (size != 50 && size != 100)
312312
return;
313313

314-
malloc(size * sizeof(int)); // GOOD [FALSE POSITIVE]
314+
malloc(size * sizeof(int)); // GOOD
315315
}
316316
}

0 commit comments

Comments
 (0)