Skip to content

Commit

Permalink
Don't pass null pointers to memcmp and memcpy in libFuzzer (llvm#96775)
Browse files Browse the repository at this point in the history
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 llvm#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 llvm#96772
  • Loading branch information
davidben committed Aug 13, 2024
1 parent 2596464 commit bde4ffe
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
4 changes: 3 additions & 1 deletion compiler-rt/lib/fuzzer/FuzzerDictionary.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
7 changes: 6 additions & 1 deletion compiler-rt/lib/fuzzer/FuzzerLoop.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
Expand Down

0 comments on commit bde4ffe

Please sign in to comment.