[compiler-rt] Work around a C language bug in libFuzzer (PR #96775)

[David Benjamin via llvm-commits]

https://github.com/davidben updated https://github.com/llvm/llvm-project/pull/96775

>From 6ddda792c86cf543f97db5358a8ebd7a0cb4ae87 Mon Sep 17 00:00:00 2001
From: David Benjamin <davidben at google.com>
Date: Wed, 26 Jun 2024 10:27:06 -0400
Subject: [PATCH] Don't pass null pointers to memcmp and memcpy in libFuzzer

In C, it is UB to call memcmp(NULL, NULL, 0), memcpy(NULL, NULL, 0),
etc. Unfortunately, (NULL, 0) is the natural representation of an empty
sequence of objects and extremely common in real world code. As a
result, all C code, and C++ code which calls into C functions, must
carefully guard all calls to memcpy.

This is a serious, real world usability issue in C and should be fixed
in the language (see #49459). In the meantime, pay the cost of the extra
branch to avoid tripping UBSan in libFuzzer. Once the usability problem
in C has been fixed, these checks can be removed.

Fixes #96772
---
 compiler-rt/lib/fuzzer/FuzzerDictionary.h | 4 +++-
 compiler-rt/lib/fuzzer/FuzzerLoop.cpp     | 7 ++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/compiler-rt/lib/fuzzer/FuzzerDictionary.h b/compiler-rt/lib/fuzzer/FuzzerDictionary.h
index 48f063c7ee4e6..64eb35c57a561 100644
--- a/compiler-rt/lib/fuzzer/FuzzerDictionary.h
+++ b/compiler-rt/lib/fuzzer/FuzzerDictionary.h
@@ -29,7 +29,9 @@ template <size_t kMaxSizeT> class FixedWord {
     static_assert(kMaxSizeT <= std::numeric_limits<uint8_t>::max(),
                   "FixedWord::kMaxSizeT cannot fit in a uint8_t.");
     assert(S <= kMaxSize);
-    memcpy(Data, B, S);
+    // memcpy cannot take null pointer arguments even if Size is 0.
+    if (S)
+      memcpy(Data, B, S);
     Size = static_cast<uint8_t>(S);
   }
 
diff --git a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
index 935dd2342e18f..6f415dd5763ac 100644
--- a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
+++ b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp
@@ -579,6 +579,9 @@ void Fuzzer::CrashOnOverwrittenData() {
 // Compare two arrays, but not all bytes if the arrays are large.
 static bool LooseMemeq(const uint8_t *A, const uint8_t *B, size_t Size) {
   const size_t Limit = 64;
+  // memcmp cannot take null pointer arguments even if Size is 0.
+  if (!Size)
+    return true;
   if (Size <= 64)
     return !memcmp(A, B, Size);
   // Compare first and last Limit/2 bytes.
@@ -596,7 +599,9 @@ ATTRIBUTE_NOINLINE bool Fuzzer::ExecuteCallback(const uint8_t *Data,
   // We copy the contents of Unit into a separate heap buffer
   // so that we reliably find buffer overflows in it.
   uint8_t *DataCopy = new uint8_t[Size];
-  memcpy(DataCopy, Data, Size);
+  // memcpy cannot take null pointer arguments even if Size is 0.
+  if (Size)
+    memcpy(DataCopy, Data, Size);
   if (EF->__msan_unpoison)
     EF->__msan_unpoison(DataCopy, Size);
   if (EF->__msan_unpoison_param)