Skip to content

Commit f267696

Browse files
MathiasVPMathiasVP
committed
C++: Actally convert 'cpp/overflow-destination' to a path-problem query.
1 parent 8a8fb69 commit f267696

File tree

3 files changed

+53
-10
lines changed

3 files changed

+53
-10
lines changed

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
* @name Copy function using source size
33
* @description Calling a copy operation with a size derived from the source
44
* buffer instead of the destination buffer may result in a buffer overflow.
5-
* @kind problem
5+
* @kind path-problem
66
* @id cpp/overflow-destination
77
* @problem.severity warning
88
* @security-severity 9.3
@@ -30,9 +30,9 @@ predicate sourceSized(FunctionCall fc, Expr src) {
3030
fc.getTarget().hasGlobalOrStdName(["strncpy", "strncat", "memcpy", "memmove"]) and
3131
exists(Expr dest, Expr size, Variable v |
3232
fc.getArgument(0) = dest and
33-
fc.getArgument(1) = src and
33+
fc.getArgument(1).getFullyConverted() = src and
3434
fc.getArgument(2) = size and
35-
src = v.getAnAccess() and
35+
src = v.getAnAccess().getFullyConverted() and
3636
size.getAChild+() = v.getAnAccess() and
3737
// exception: `dest` is also referenced in the size argument
3838
not exists(Variable other |
@@ -71,7 +71,7 @@ class OverflowDestinationConfig extends TaintTracking::Configuration {
7171

7272
override predicate isSource(DataFlow::Node source) { source instanceof FlowSource }
7373

74-
override predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asExpr()) }
74+
override predicate isSink(DataFlow::Node sink) { sourceSized(_, sink.asConvertedExpr()) }
7575

7676
override predicate isSanitizer(DataFlow::Node node) {
7777
exists(Variable checkedVar |
@@ -91,6 +91,6 @@ from
9191
DataFlow::PathNode sink
9292
where
9393
conf.hasFlowPath(source, sink) and
94-
sourceSized(fc, sink.getNode().asExpr())
95-
select fc,
94+
sourceSized(fc, sink.getNode().asConvertedExpr())
95+
select fc, source, sink,
9696
"To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size."

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/SAMATE/OverflowDestination.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
edges
2+
nodes
3+
subpaths
4+
#select

‎cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,43 @@
1-
| overflowdestination.cpp:30:2:30:8 | call to strncpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
2-
| overflowdestination.cpp:46:2:46:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
3-
| overflowdestination.cpp:53:2:53:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
4-
| overflowdestination.cpp:64:2:64:7 | call to memcpy | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
1+
edges
2+
| overflowdestination.cpp:27:9:27:12 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... |
3+
| overflowdestination.cpp:43:8:43:10 | fgets output argument | overflowdestination.cpp:46:15:46:17 | (const void *)... |
4+
| overflowdestination.cpp:50:52:50:54 | *src | overflowdestination.cpp:50:52:50:54 | ReturnIndirection |
5+
| overflowdestination.cpp:50:52:50:54 | src | overflowdestination.cpp:53:15:53:17 | (const void *)... |
6+
| overflowdestination.cpp:57:52:57:54 | *src | overflowdestination.cpp:64:16:64:19 | (const void *)... |
7+
| overflowdestination.cpp:57:52:57:54 | src | overflowdestination.cpp:64:16:64:19 | (const void *)... |
8+
| overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:75:30:75:32 | src |
9+
| overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:75:30:75:32 | src indirection |
10+
| overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:76:30:76:32 | src |
11+
| overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:76:30:76:32 | src indirection |
12+
| overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument | overflowdestination.cpp:76:30:76:32 | src |
13+
| overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument | overflowdestination.cpp:76:30:76:32 | src indirection |
14+
| overflowdestination.cpp:75:30:75:32 | src | overflowdestination.cpp:50:52:50:54 | src |
15+
| overflowdestination.cpp:75:30:75:32 | src indirection | overflowdestination.cpp:50:52:50:54 | *src |
16+
| overflowdestination.cpp:75:30:75:32 | src indirection | overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument |
17+
| overflowdestination.cpp:76:30:76:32 | src | overflowdestination.cpp:57:52:57:54 | src |
18+
| overflowdestination.cpp:76:30:76:32 | src indirection | overflowdestination.cpp:57:52:57:54 | *src |
19+
nodes
20+
| overflowdestination.cpp:27:9:27:12 | argv | semmle.label | argv |
21+
| overflowdestination.cpp:30:17:30:20 | (const char *)... | semmle.label | (const char *)... |
22+
| overflowdestination.cpp:43:8:43:10 | fgets output argument | semmle.label | fgets output argument |
23+
| overflowdestination.cpp:46:15:46:17 | (const void *)... | semmle.label | (const void *)... |
24+
| overflowdestination.cpp:50:52:50:54 | *src | semmle.label | *src |
25+
| overflowdestination.cpp:50:52:50:54 | ReturnIndirection | semmle.label | ReturnIndirection |
26+
| overflowdestination.cpp:50:52:50:54 | src | semmle.label | src |
27+
| overflowdestination.cpp:53:15:53:17 | (const void *)... | semmle.label | (const void *)... |
28+
| overflowdestination.cpp:57:52:57:54 | *src | semmle.label | *src |
29+
| overflowdestination.cpp:57:52:57:54 | src | semmle.label | src |
30+
| overflowdestination.cpp:64:16:64:19 | (const void *)... | semmle.label | (const void *)... |
31+
| overflowdestination.cpp:73:8:73:10 | fgets output argument | semmle.label | fgets output argument |
32+
| overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument | semmle.label | overflowdest_test2 output argument |
33+
| overflowdestination.cpp:75:30:75:32 | src | semmle.label | src |
34+
| overflowdestination.cpp:75:30:75:32 | src indirection | semmle.label | src indirection |
35+
| overflowdestination.cpp:76:30:76:32 | src | semmle.label | src |
36+
| overflowdestination.cpp:76:30:76:32 | src indirection | semmle.label | src indirection |
37+
subpaths
38+
| overflowdestination.cpp:75:30:75:32 | src indirection | overflowdestination.cpp:50:52:50:54 | *src | overflowdestination.cpp:50:52:50:54 | ReturnIndirection | overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument |
39+
#select
40+
| overflowdestination.cpp:30:2:30:8 | call to strncpy | overflowdestination.cpp:27:9:27:12 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
41+
| overflowdestination.cpp:46:2:46:7 | call to memcpy | overflowdestination.cpp:43:8:43:10 | fgets output argument | overflowdestination.cpp:46:15:46:17 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
42+
| overflowdestination.cpp:53:2:53:7 | call to memcpy | overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:53:15:53:17 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
43+
| overflowdestination.cpp:64:2:64:7 | call to memcpy | overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:64:16:64:19 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |

0 commit comments

Comments
 (0)