Merge pull request github#13929 from jketema/buffer

C++: Only consider the maximum buffer size for badly bounded write

Author: jketema

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() +

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.

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. |

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
+}