Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++: Improve and promote cpp/overflow-buffer #18837

Merged
merged 20 commits into from
Mar 3, 2025
Merged

C++: Improve and promote cpp/overflow-buffer #18837

Changes from 7 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e77ebf0
C++: Test spacing.
geoffw0 Feb 20, 2025
8935599
C++: Additional test cases.
geoffw0 Feb 20, 2025
3aa1ba5
C++: More additional test cases.
geoffw0 Feb 20, 2025
547b082
C++: Even more test cases.
geoffw0 Feb 20, 2025
ae25399
C++: Fix offsetof bug.
geoffw0 Feb 20, 2025
fd32355
C++: Give cpp/overflow-buffer medium precision, and upgrade severity …
geoffw0 Feb 20, 2025
90758b3
C++: Change notes.
geoffw0 Feb 20, 2025
3681ace
C++: Explore negative indices more in tests.
geoffw0 Feb 24, 2025
07004bd
C++: Test cases motivated by a real world FP.
geoffw0 Feb 25, 2025
812315d
C++: Use existing getSize / getRootType to find more generous bounds …
geoffw0 Feb 25, 2025
7d7498e
C++: Avoid problems when a variable has multiple types.
geoffw0 Feb 25, 2025
a7ac6b1
C++: Second change note.
geoffw0 Feb 25, 2025
c41add8
C++: Accept regressions in SAMATE based test cases.
geoffw0 Feb 26, 2025
abb88e3
C++: Add a test file that was internal (results as on main).
geoffw0 Feb 26, 2025
dbab845
C++: Effect of this branch on the new test.
geoffw0 Feb 26, 2025
1354beb
C++: Fix an issue with padding.
geoffw0 Feb 26, 2025
3d19e2a
C++: Static buffer overflow change note.
geoffw0 Feb 27, 2025
51d9162
C++: Another test.
geoffw0 Feb 27, 2025
998bec1
C++: Fix the bug.
geoffw0 Feb 27, 2025
7169c4b
C++: Another attempt to make the fix more solid. I believe it can't …
geoffw0 Feb 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cpp/ql/lib/change-notes/2025-02-20-getbuffersize.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fixed an issue where the `getBufferSize` predicate in `commons/Buffer.qll` was returning results for references inside `offsetof` expressions, which are not accesses to a buffer.
1 change: 1 addition & 0 deletions cpp/ql/lib/semmle/code/cpp/commons/Buffer.qll
View file
Original file line number Diff line number Diff line change
@@ -108,6 +108,7 @@ private int isSource(Expr bufferExpr, Element why) {
result = bufferVar.getUnspecifiedType().(ArrayType).getSize() and
why = bufferVar and
not memberMayBeVarSize(_, bufferVar) and
not exists(BuiltInOperationBuiltInOffsetOf offsetof | offsetof.getAChild*() = bufferExpr) and
// zero sized arrays are likely to have special usage, for example
// behaving a bit like a 'union' overlapping other fields.
not result = 0
3 changes: 2 additions & 1 deletion cpp/ql/src/Security/CWE/CWE-119/OverflowBuffer.ql
View file
Original file line number Diff line number Diff line change
@@ -5,8 +5,9 @@
* buffer.
* @kind problem
* @id cpp/overflow-buffer
* @problem.severity recommendation
* @problem.severity warning
* @security-severity 9.3
* @precision medium
* @tags security
* external/cwe/cwe-119
* external/cwe/cwe-121
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2025-02-20-overflow-buffer.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The query "Call to memory access function may overflow buffer" (`cpp/overflow-buffer`) has been added to the security-extended query suite. The query detects a range of buffer overflow and underflow issues.
142 changes: 81 additions & 61 deletions cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowBuffer.expected
View file

Large diffs are not rendered by default.

8 changes: 6 additions & 2 deletions cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowStatic.expected
View file
Original file line number Diff line number Diff line change
@@ -3,6 +3,10 @@
| tests.cpp:163:3:163:11 | access to array | Potential buffer-overflow: counter 'k' <= 100 but 'buffer' has 100 elements. |
| tests.cpp:164:8:164:16 | access to array | Potential buffer-overflow: counter 'k' <= 100 but 'buffer' has 100 elements. |
| tests.cpp:245:42:245:42 | 6 | Potential buffer-overflow: 'global_array_5' has size 5 not 6. |
| tests.cpp:349:2:349:14 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
| tests.cpp:350:17:350:29 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
| tests.cpp:351:2:351:14 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
| tests.cpp:352:17:352:29 | access to array | Potential buffer-overflow: 'charArray' has size 10 but 'charArray[10]' may be accessed here. |
| tests.cpp:927:2:927:16 | access to array | Potential buffer-overflow: 'cs' has size 4 but 'cs[4]' may be accessed here. |
| tests.cpp:928:2:928:17 | access to array | Potential buffer-overflow: 'cs' has size 4 but 'cs[39]' may be accessed here. |
| tests.cpp:929:2:929:17 | access to array | Potential buffer-overflow: 'cs' has size 4 but 'cs[40]' may be accessed here. |
| tests.cpp:932:2:932:16 | access to array | Potential buffer-overflow: 'cs' has size 4 but 'cs[4]' may be accessed here. |
| var_size_struct.cpp:103:39:103:41 | 129 | Potential buffer-overflow: 'str' has size 128 not 129. |
58 changes: 29 additions & 29 deletions cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected
View file
Original file line number Diff line number Diff line change
@@ -27,26 +27,26 @@ edges
| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance | |
| main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | **argv | provenance | |
| main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | *argv | provenance | |
| main.cpp:10:20:10:23 | **argv | tests.cpp:872:32:872:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:872:32:872:35 | *argv | provenance | |
| main.cpp:10:20:10:23 | **argv | tests.cpp:978:32:978:35 | **argv | provenance | |
| main.cpp:10:20:10:23 | *argv | tests.cpp:978:32:978:35 | *argv | provenance | |
| overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | provenance | |
| overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | provenance | |
| test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance | |
| test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | provenance | |
| test_buffer_overrun.cpp:32:46:32:49 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | provenance | |
| tests.cpp:613:19:613:24 | *source | tests.cpp:615:17:615:22 | *source | provenance | |
| tests.cpp:622:19:622:24 | *source | tests.cpp:625:2:625:16 | *... = ... | provenance | |
| tests.cpp:625:2:625:2 | *s [post update] [*home] | tests.cpp:628:14:628:14 | *s [*home] | provenance | |
| tests.cpp:625:2:625:16 | *... = ... | tests.cpp:625:2:625:2 | *s [post update] [*home] | provenance | |
| tests.cpp:628:14:628:14 | *s [*home] | tests.cpp:628:14:628:19 | *home | provenance | |
| tests.cpp:628:14:628:14 | *s [*home] | tests.cpp:628:16:628:19 | *home | provenance | |
| tests.cpp:628:16:628:19 | *home | tests.cpp:628:14:628:19 | *home | provenance | |
| tests.cpp:872:32:872:35 | **argv | tests.cpp:897:9:897:15 | *access to array | provenance | |
| tests.cpp:872:32:872:35 | **argv | tests.cpp:898:9:898:15 | *access to array | provenance | |
| tests.cpp:872:32:872:35 | *argv | tests.cpp:897:9:897:15 | *access to array | provenance | |
| tests.cpp:872:32:872:35 | *argv | tests.cpp:898:9:898:15 | *access to array | provenance | |
| tests.cpp:897:9:897:15 | *access to array | tests.cpp:613:19:613:24 | *source | provenance | |
| tests.cpp:898:9:898:15 | *access to array | tests.cpp:622:19:622:24 | *source | provenance | |
| tests.cpp:634:19:634:24 | *source | tests.cpp:636:17:636:22 | *source | provenance | |
| tests.cpp:643:19:643:24 | *source | tests.cpp:646:2:646:16 | *... = ... | provenance | |
| tests.cpp:646:2:646:2 | *s [post update] [*home] | tests.cpp:649:14:649:14 | *s [*home] | provenance | |
| tests.cpp:646:2:646:16 | *... = ... | tests.cpp:646:2:646:2 | *s [post update] [*home] | provenance | |
| tests.cpp:649:14:649:14 | *s [*home] | tests.cpp:649:14:649:19 | *home | provenance | |
| tests.cpp:649:14:649:14 | *s [*home] | tests.cpp:649:16:649:19 | *home | provenance | |
| tests.cpp:649:16:649:19 | *home | tests.cpp:649:14:649:19 | *home | provenance | |
| tests.cpp:978:32:978:35 | **argv | tests.cpp:1003:9:1003:15 | *access to array | provenance | |
| tests.cpp:978:32:978:35 | **argv | tests.cpp:1004:9:1004:15 | *access to array | provenance | |
| tests.cpp:978:32:978:35 | *argv | tests.cpp:1003:9:1003:15 | *access to array | provenance | |
| tests.cpp:978:32:978:35 | *argv | tests.cpp:1004:9:1004:15 | *access to array | provenance | |
| tests.cpp:1003:9:1003:15 | *access to array | tests.cpp:634:19:634:24 | *source | provenance | |
| tests.cpp:1004:9:1004:15 | *access to array | tests.cpp:643:19:643:24 | *source | provenance | |
| tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | provenance | |
| tests_restrict.c:15:41:15:44 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance | |
nodes
@@ -72,18 +72,18 @@ nodes
| test_buffer_overrun.cpp:32:46:32:49 | *argv | semmle.label | *argv |
| test_buffer_overrun.cpp:32:46:32:49 | *argv | semmle.label | *argv |
| test_buffer_overrun.cpp:32:46:32:49 | *argv | semmle.label | *argv |
| tests.cpp:613:19:613:24 | *source | semmle.label | *source |
| tests.cpp:615:17:615:22 | *source | semmle.label | *source |
| tests.cpp:622:19:622:24 | *source | semmle.label | *source |
| tests.cpp:625:2:625:2 | *s [post update] [*home] | semmle.label | *s [post update] [*home] |
| tests.cpp:625:2:625:16 | *... = ... | semmle.label | *... = ... |
| tests.cpp:628:14:628:14 | *s [*home] | semmle.label | *s [*home] |
| tests.cpp:628:14:628:19 | *home | semmle.label | *home |
| tests.cpp:628:16:628:19 | *home | semmle.label | *home |
| tests.cpp:872:32:872:35 | **argv | semmle.label | **argv |
| tests.cpp:872:32:872:35 | *argv | semmle.label | *argv |
| tests.cpp:897:9:897:15 | *access to array | semmle.label | *access to array |
| tests.cpp:898:9:898:15 | *access to array | semmle.label | *access to array |
| tests.cpp:634:19:634:24 | *source | semmle.label | *source |
| tests.cpp:636:17:636:22 | *source | semmle.label | *source |
| tests.cpp:643:19:643:24 | *source | semmle.label | *source |
| tests.cpp:646:2:646:2 | *s [post update] [*home] | semmle.label | *s [post update] [*home] |
| tests.cpp:646:2:646:16 | *... = ... | semmle.label | *... = ... |
| tests.cpp:649:14:649:14 | *s [*home] | semmle.label | *s [*home] |
| tests.cpp:649:14:649:19 | *home | semmle.label | *home |
| tests.cpp:649:16:649:19 | *home | semmle.label | *home |
| tests.cpp:978:32:978:35 | **argv | semmle.label | **argv |
| tests.cpp:978:32:978:35 | *argv | semmle.label | *argv |
| tests.cpp:1003:9:1003:15 | *access to array | semmle.label | *access to array |
| tests.cpp:1004:9:1004:15 | *access to array | semmle.label | *access to array |
| tests_restrict.c:15:41:15:44 | **argv | semmle.label | **argv |
| tests_restrict.c:15:41:15:44 | **argv | semmle.label | **argv |
| tests_restrict.c:15:41:15:44 | *argv | semmle.label | *argv |
@@ -97,5 +97,5 @@ subpaths
| main.cpp:9:29:9:32 | **argv | tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument |
| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | tests_restrict.c:15:41:15:44 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument |
#select
| tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:615:17:615:22 | *source | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |
| tests.cpp:628:2:628:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:628:14:628:19 | *home | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |
| tests.cpp:636:2:636:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:636:17:636:22 | *source | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |
| tests.cpp:649:2:649:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:649:14:649:19 | *home | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |
Loading
Oops, something went wrong.