[PATCH] D159108: [analyzer] CStringChecker should check the first byte of the destination of strcpy, strncpy

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, xazax.hun, donat.nagy, Szelethus.
Herald added subscribers: manas, ASDenysPetrov, martong, dkrupp, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

By not checking if the first byte of the destination of strcpy and
strncpy is writable, we missed some reports in the Juliet benchmark.

(Juliet CWE-124 Buffer Underwrite: strcpy, strncpy)

Depends on D159107 <https://reviews.llvm.org/D159107>


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159108

Files:
  clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
  clang/test/Analysis/string.c


Index: clang/test/Analysis/string.c
===================================================================
--- clang/test/Analysis/string.c
+++ clang/test/Analysis/string.c
@@ -1667,3 +1667,49 @@
   strcpy(x, "12\0");
 }
 #endif
+
+#ifndef SUPPRESS_OUT_OF_BOUND
+void testStrcpyDestinationWritableFirstByte(void) {
+  char dst[10];
+  char *p = dst - 8;
+  strcpy(p, "src"); // expected-warning {{String copy function overflows the destination buffer}}
+}
+
+void CWE124_Buffer_Underwrite__malloc_char_cpy() {
+  char * dataBuffer = (char *)malloc(100*sizeof(char));
+  if (dataBuffer == NULL) return;
+  memset(dataBuffer, 'A', 100-1);
+  dataBuffer[100-1] = '\0';
+  char * data = dataBuffer - 8;
+  char source[100];
+  memset(source, 'C', 100-1); // fill with 'C's
+  source[100-1] = '\0'; // null terminate
+  strcpy(data, source); // expected-warning {{String copy function overflows the destination buffer}}
+  free(dataBuffer);
+}
+#endif
+
+#ifndef SUPPRESS_OUT_OF_BOUND
+void testStrncpyDestinationWritableFirstByte(void) {
+  char source[100];
+  use_string(source); // escape
+  char buf[100];
+  char *p = buf - 8;
+  strncpy(p, source, 100-1); // expected-warning {{String copy function overflows the destination buffer}}
+}
+
+void CWE124_Buffer_Underwrite__malloc_char_ncpy() {
+  char * dataBuffer = (char *)malloc(100*sizeof(char));
+  if (dataBuffer == 0) return;
+  memset(dataBuffer, 'A', 100-1);
+  dataBuffer[100-1] = '\0';
+  char *data = dataBuffer - 8;
+
+  char source[100];
+  memset(source, 'C', 100-1); // fill with 'C's
+  source[100-1] = '\0'; // null terminate
+  strncpy(data, source, 100-1); // expected-warning {{String copy function overflows the destination buffer}}
+  data[100-1] = '\0'; // null terminate
+  free(dataBuffer);
+}
+#endif
Index: clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp
@@ -2009,6 +2009,11 @@
       SVal maxLastElement =
           svalBuilder.evalBinOpLN(state, BO_Add, *dstRegVal, *maxLastNL, ptrTy);
 
+      // Check if the first byte of the destination is writable.
+      state = CheckLocation(C, state, Dst, DstVal, AccessKind::write);
+      if (!state)
+        return;
+      // Check if the last byte of the destination is writable.
       state = CheckLocation(C, state, Dst, maxLastElement, AccessKind::write);
       if (!state)
         return;
@@ -2021,6 +2026,11 @@
 
       // ...and we haven't checked the bound, we'll check the actual copy.
       if (!boundWarning) {
+        // Check if the first byte of the destination is writable.
+        state = CheckLocation(C, state, Dst, DstVal, AccessKind::write);
+        if (!state)
+          return;
+        // Check if the last byte of the destination is writable.
         state = CheckLocation(C, state, Dst, lastElement, AccessKind::write);
         if (!state)
           return;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D159108.554354.patch
Type: text/x-patch
Size: 3005 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230829/69f063ba/attachment.bin>