From bde4ffe7521421cfa891c7d6e526566920326b3f Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 13 Aug 2024 18:26:40 -0400 Subject: [PATCH] Don't pass null pointers to memcmp and memcpy in libFuzzer (#96775) 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 48f063c7ee4e6e..64eb35c57a5610 100644 --- a/compiler-rt/lib/fuzzer/FuzzerDictionary.h +++ b/compiler-rt/lib/fuzzer/FuzzerDictionary.h @@ -29,7 +29,9 @@ template class FixedWord { static_assert(kMaxSizeT <= std::numeric_limits::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(S); } diff --git a/compiler-rt/lib/fuzzer/FuzzerLoop.cpp b/compiler-rt/lib/fuzzer/FuzzerLoop.cpp index 935dd2342e18f8..6f415dd5763ac9 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)