diff --git a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql index 247606c683d..e7dd6a5d8e3 100644 --- a/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql +++ b/cpp/ql/src/Security/CWE/CWE-120/BadlyBoundedWrite.ql @@ -24,7 +24,7 @@ import semmle.code.cpp.security.BufferWrite from BufferWrite bw, int destSize where bw.hasExplicitLimit() and // has an explicit size limit - destSize = getBufferSize(bw.getDest(), _) and + destSize = max(getBufferSize(bw.getDest(), _)) and bw.getExplicitLimit() > destSize // but it's larger than the destination select bw, "This '" + bw.getBWDesc() + "' operation is limited to " + bw.getExplicitLimit() + diff --git a/cpp/ql/src/change-notes/2023-08-09-badly-bounded-write.md b/cpp/ql/src/change-notes/2023-08-09-badly-bounded-write.md new file mode 100644 index 00000000000..1dd4705754b --- /dev/null +++ b/cpp/ql/src/change-notes/2023-08-09-badly-bounded-write.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The `cpp/badly-bounded-write` query could report false positives when a pointer was first initialized with a literal and later assigned a dynamically allocated array. These false positives now no longer occur. diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected index c20cf040504..022ae91391e 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/VeryLikelyOverrunWrite.expected @@ -1,10 +1,10 @@ -| tests2.cpp:17:3:17:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. | -| tests2.cpp:22:3:22:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. | -| tests2.cpp:27:3:27:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. | -| tests2.cpp:31:3:31:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. | -| tests2.cpp:36:3:36:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. | -| tests2.cpp:41:3:41:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. | -| tests2.cpp:46:3:46:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. | +| tests2.cpp:18:3:18:8 | call to wcscpy | This 'call to wcscpy' operation requires 12 bytes but the destination is only 8 bytes. | +| tests2.cpp:23:3:23:8 | call to wcscpy | This 'call to wcscpy' operation requires 16 bytes but the destination is only 12 bytes. | +| tests2.cpp:28:3:28:8 | call to wcscpy | This 'call to wcscpy' operation requires 20 bytes but the destination is only 16 bytes. | +| tests2.cpp:32:3:32:8 | call to wcscpy | This 'call to wcscpy' operation requires 24 bytes but the destination is only 20 bytes. | +| tests2.cpp:37:3:37:8 | call to wcscpy | This 'call to wcscpy' operation requires 28 bytes but the destination is only 24 bytes. | +| tests2.cpp:42:3:42:8 | call to wcscpy | This 'call to wcscpy' operation requires 32 bytes but the destination is only 28 bytes. | +| tests2.cpp:47:3:47:8 | call to wcscpy | This 'call to wcscpy' operation requires 36 bytes but the destination is only 32 bytes. | | tests.c:54:3:54:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. | | tests.c:58:3:58:9 | call to sprintf | This 'call to sprintf' operation requires 11 bytes but the destination is only 10 bytes. | | tests.c:62:17:62:24 | buffer10 | This 'scanf string argument' operation requires 11 bytes but the destination is only 10 bytes. | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp index 6081191917d..97699982800 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-120/semmle/tests/tests2.cpp @@ -6,6 +6,7 @@ void *realloc(void *ptr, size_t size); void *calloc(size_t nmemb, size_t size); void free(void *ptr); wchar_t *wcscpy(wchar_t *s1, const wchar_t *s2); +int snprintf(char *s, size_t n, const char *format, ...); // --- Semmle tests --- @@ -46,3 +47,18 @@ void tests2() { wcscpy(buffer, L"12345678"); // BAD: buffer overflow delete [] buffer; } + +char* dest1 = "a"; +char* dest2 = "abcdefghijklmnopqrstuvwxyz"; + +void test3() { + const char src[] = "abcdefghijkl"; + dest1 = (char*)malloc(sizeof(src)); + if (!dest1) + return; + snprintf(dest1, sizeof(src), "%s", src); // GOOD + dest2 = (char*)malloc(3); + if (!dest2) + return; + snprintf(dest2, sizeof(src), "%s", src); // BAD [NOT DETECTED]: buffer overflow +}