[analyzer] CStringChecker buffer access checks should check the first bytes

[steakhal]

Summary

By not checking if the first byte of the buffer is accessible,
we missed some reports in the Juliet benchmark.

(Juliet CWE-124 Buffer Underwrite: memcpy, memmove)

Depends on D159108

[donat.nagy]

LGTM if the test results are also good.

[xazax.hun]

LG!

clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp

CStringChecker::CheckBufferAccess(CheckerContext &C, ProgramStateRef State,
   State = checkNonNull(C, State, Buffer, BufVal);
   if (!State)
     return nullptr;
 
   // If out-of-bounds checking is turned off, skip the rest.
   if (!Filter.CheckCStringOutOfBounds)
     return State;
 
+  SVal BufStart =
+      svalBuilder.evalCast(BufVal, PtrTy, Buffer.Expression->getType());
+
+  // Check if the first byte of the buffer is accessible.
+  State = CheckLocation(C, State, Buffer, BufStart, Access, CK);
+  if (!State)
+    return nullptr;
+
   // Get the access length and make sure it is known.
   // FIXME: This assumes the caller has already checked that the access length
   // is positive. And that it's unsigned.
   SVal LengthVal = C.getSVal(Size.Expression);
   std::optional<NonLoc> Length = LengthVal.getAs<NonLoc>();
   if (!Length)
     return State;
 
   // Compute the offset of the last element to be accessed: size-1.
   NonLoc One = svalBuilder.makeIntVal(1, SizeTy).castAs<NonLoc>();
   SVal Offset = svalBuilder.evalBinOpNN(State, BO_Sub, *Length, One, SizeTy);
   if (Offset.isUnknown())
     return nullptr;
   NonLoc LastOffset = Offset.castAs<NonLoc>();
 
   // Check that the first buffer is sufficiently long.
-  SVal BufStart =
-      svalBuilder.evalCast(BufVal, PtrTy, Buffer.Expression->getType());
   if (std::optional<Loc> BufLoc = BufStart.getAs<Loc>()) {
 
     SVal BufEnd =
         svalBuilder.evalBinOpLN(State, BO_Add, *BufLoc, LastOffset, PtrTy);
     State = CheckLocation(C, State, Buffer, BufEnd, Access, CK);
 
     // If the buffer isn't large enough, abort.
     if (!State)

clang/test/Analysis/string.c

 #define NULL 0
 typedef typeof(sizeof(int)) size_t;
 
 void clang_analyzer_eval(int);
 
 int scanf(const char *restrict format, ...);
 void *malloc(size_t);
 void free(void *);
+void *memcpy(void *dest, const void *src, unsigned long n);
 
 //===----------------------------------------------------------------------===
 // strlen()
 //===----------------------------------------------------------------------===
 
 #define strlen BUILTIN(strlen)
 size_t strlen(const char *s);
 
void CWE124_Buffer_Underwrite__malloc_char_ncpy() {
   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
+
+#ifndef SUPPRESS_OUT_OF_BOUND
+void CWE124_Buffer_Underwrite__malloc_char_memcpy() {
+  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
+
+  memcpy(data, source, 100*sizeof(char)); // expected-warning {{Memory copy function overflows the destination buffer}}
+  data[100-1] = '\0';
+  free(dataBuffer);
+}
+#endif