Skip to content

Commit a005920

Browse files
authored
C++: split cpp/overrunning-write into two
This splits the `cpp/overruning-write` into two separate queries based off on the reason for the estimation. If the overrun is detected based on non-trivial range analysis, the results are now marked by the new `cpp/very-likely-overruning-write` high precision query. If it is based on less precise, usually type based bounds, then it will still be marked by `cpp/overruning-write` which remains at medium precision.
1 parent 40ad88b commit a005920

File tree

12 files changed

+75
-37
lines changed

12 files changed

+75
-37
lines changed

cpp/ql/src/Security/CWE/CWE-120/OverrunWrite.ql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,12 @@ import semmle.code.cpp.commons.Alloc
2121
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
2222
*/
2323

24-
from BufferWrite bw, Expr dest, int destSize, int estimated
24+
from BufferWrite bw, Expr dest, int destSize, int estimated, TypeBoundsAnalysis reason
2525
where
2626
not bw.hasExplicitLimit() and // has no explicit size limit
2727
dest = bw.getDest() and
2828
destSize = getBufferSize(dest, _) and
29-
estimated = bw.getMaxDataLimited(_) and
29+
estimated = bw.getMaxDataLimited(reason) and
3030
// we can deduce that too much data may be copied (even without
3131
// long '%f' conversions)
3232
estimated > destSize
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* @name Likely overrunning write based on non-trivial analysis of value ranges
3+
* @description Buffer write operations that do not control the length
4+
* of data written may overflow
5+
* @kind problem
6+
* @problem.severity error
7+
* @security-severity 9.3
8+
* @precision high
9+
* @id cpp/very-likely-overrunning-write
10+
* @tags reliability
11+
* security
12+
* external/cwe/cwe-120
13+
* external/cwe/cwe-787
14+
* external/cwe/cwe-805
15+
*/
16+
17+
import semmle.code.cpp.security.BufferWrite
18+
import semmle.code.cpp.commons.Alloc
19+
20+
/*
21+
* See CWE-120/UnboundedWrite.ql for a summary of CWE-120 alert cases.
22+
*/
23+
24+
from BufferWrite bw, Expr dest, int destSize, int estimated, ValueFlowAnalysis reason
25+
where
26+
not bw.hasExplicitLimit() and // has no explicit size limit
27+
dest = bw.getDest() and
28+
destSize = getBufferSize(dest, _) and
29+
estimated = bw.getMaxDataLimited(reason) and
30+
// we can deduce from non-trivial range analysis that too much data may be copied
31+
estimated > destSize
32+
select bw,
33+
"This '" + bw.getBWDesc() + "' operation requires " + estimated +
34+
" bytes but the destination is only " + destSize + " bytes."

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

Whitespace-only changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql

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

Whitespace-only changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +0,0 @@
1-
| tests2.cpp:17:3:17:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. |
2-
| tests2.cpp:22:3:22:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. |
3-
| tests2.cpp:27:3:27:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. |
4-
| tests2.cpp:31:3:31:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. |
5-
| tests2.cpp:36:3:36:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. |
6-
| tests2.cpp:41:3:41:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. |
7-
| tests2.cpp:46:3:46:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. |
8-
| tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
9-
| tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
10-
| tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
11-
| tests.c:63:17:63:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
12-
| tests.c:86:3:86:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
13-
| tests.c:93:3:93:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
14-
| tests.c:120:3:120:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
15-
| tests.c:121:3:121:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
16-
| tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
17-
| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
18-
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
19-
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
20-
| unions.c:32:2:32:7 | call to strcpy | This 'call to strcpy' operation requires 31 bytes but the destination is only 25 bytes. |
21-
| var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
| tests2.cpp:17:3:17:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. |
2+
| tests2.cpp:22:3:22:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. |
3+
| tests2.cpp:27:3:27:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. |
4+
| tests2.cpp:31:3:31:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. |
5+
| tests2.cpp:36:3:36:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. |
6+
| tests2.cpp:41:3:41:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. |
7+
| tests2.cpp:46:3:46:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. |
8+
| tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
9+
| tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
10+
| tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. |
11+
| tests.c:63:17:63:24 | buffer10 | This 'scanf string argument' operation requires 12 bytes but the destination is only 10 bytes. |
12+
| tests.c:86:3:86:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
13+
| tests.c:93:3:93:8 | call to strcpy | This 'call to strcpy' operation requires 6 bytes but the destination is only 5 bytes. |
14+
| tests.c:120:3:120:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 1 bytes. |
15+
| tests.c:121:3:121:9 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 16 bytes. |
16+
| tests.c:136:2:136:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. |
17+
| unions.c:26:2:26:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
18+
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 15 bytes. |
19+
| unions.c:27:2:27:7 | call to strcpy | This 'call to strcpy' operation requires 21 bytes but the destination is only 16 bytes. |
20+
| unions.c:32:2:32:7 | call to strcpy | This 'call to strcpy' operation requires 31 bytes but the destination is only 25 bytes. |
21+
| var_size_struct.cpp:22:3:22:8 | call to strcpy | This 'call to strcpy' operation requires 10 bytes but the destination is only 9 bytes. |
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-120/VeryLikelyOverrunWrite.ql
Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,5 @@
11
| tests.cpp:258:2:258:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
22
| tests.cpp:259:2:259:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
3-
| tests.cpp:272:2:272:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
4-
| tests.cpp:273:2:273:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
5-
| tests.cpp:308:3:308:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
63
| tests.cpp:315:2:315:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
74
| tests.cpp:316:2:316:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
8-
| tests.cpp:321:2:321:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
9-
| tests.cpp:324:3:324:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 4 bytes. |
105
| tests.cpp:327:2:327:8 | call to sprintf | This 'call to sprintf' operation requires 12 bytes but the destination is only 4 bytes. |
11-
| tests.cpp:329:3:329:9 | call to sprintf | This 'call to sprintf' operation requires 12 bytes but the destination is only 4 bytes. |
12-
| tests.cpp:341:2:341:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
13-
| tests.cpp:343:2:343:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
14-
| tests.cpp:345:2:345:8 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 2 bytes. |
15-
| tests.cpp:347:2:347:8 | call to sprintf | This 'call to sprintf' operation requires 3 bytes but the destination is only 2 bytes. |
16-
| tests.cpp:350:2:350:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
17-
| tests.cpp:354:2:354:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
18-
| tests.cpp:358:2:358:8 | call to sprintf | This 'call to sprintf' operation requires 4 bytes but the destination is only 3 bytes. |
19-
| tests.cpp:363:2:363:8 | call to sprintf | This 'call to sprintf' operation requires 5 bytes but the destination is only 4 bytes. |

0 commit comments

Comments
 (0)