[LibFuzzer] Tweak `MutationDispatcher::Mutate_CopyPart` mutation.

[delcypher]

Summary

[LibFuzzer] Tweak MutationDispatcher::Mutate_CopyPart mutation.

It doesn't make sense to non-deterministically choose between
CopyPart(..) and InsertPart(..) when it is known that
InsertPart(..) will fail.

This upstream's a change from JFS solver's fork of LibFuzzer.

[kcc]

LGTM
I wonder how you can observe the change?
It's just a slight change in probabilities.
Or not slight?

I prefer to test similar changes in lib/fuzzer/tests/FuzzerUnittest.cpp
but I don't see how to test this one...

[delcypher]

In D45693#1069081, @kcc wrote:

LGTM
I wonder how you can observe the change?
It's just a slight change in probabilities.
Or not slight?

To give some context, this patch is actually part of a larger change made to JFS's copy of LibFuzzer. For JFS, inputs never change size, so it's a waste of time to perform mutations that change the input size.
To fix this in JFS we added a command line option to disable all mutations that change the input size. I don't think our approach is general enough to be upstreamed. However the change in this patch
seemed general enough to be upstreamed. At some point I plan to start a discussion on how to fix JFS's problem in a more general way on the LibFuzzer mailing list. I just haven't got around to posting yet.

I prefer to test similar changes in lib/fuzzer/tests/FuzzerUnittest.cpp
but I don't see how to test this one...

A possible way to test this might be to call Mutate_CopyPart directly with a input that is MaxSize in size and make sure that it returns MaxSize (i.e. input size is not changed).

[kcc]

Ok, sounds good.
Yes, please add a test that calls Mutate_CopyPart many times and always sees it return MaxSize

[delcypher]

Add unit test.

[delcypher]

In D45693#1071306, @kcc wrote:

Ok, sounds good.
Yes, please add a test that calls Mutate_CopyPart many times and always sees it return MaxSize

@kcc: I've added a test. Is this good enough?

[kcc]

LGTM with a nit.

lib/fuzzer/tests/FuzzerUnittest.cpp

394

Why unsigned int?

This code uses 'int' or 'size_t', depending on the context.
size_t for something that is a size, 'int' for just iterations.

unsigned int is longer to spell and is inconsistent.

[delcypher]

lib/fuzzer/tests/FuzzerUnittest.cpp

394

The only reason was I was worried about someone writing the loop bound to be so large that it became negative when using int. This isn't is a particularly good reason because there are a plethora of other ways that someone can still screw up using unsigned int so I'll switch to using int and then commit.

[delcypher]

s/unsigned int/int/ in test case.

lib/fuzzer/FuzzerMutate.cpp

if (To == From) {
     memmove(To + ToInsertPos, From + FromBeg, CopySize);
   }
   return ToSize + CopySize;
 }
 
 size_t MutationDispatcher::Mutate_CopyPart(uint8_t *Data, size_t Size,
                                            size_t MaxSize) {
   if (Size > MaxSize || Size == 0) return 0;
-  if (Rand.RandBool())
+  // If Size == MaxSize, `InsertPartOf(...)` will
+  // fail so there's no point using it in this case.
+  if (Size == MaxSize || Rand.RandBool())
     return CopyPartOf(Data, Size, Data, Size);
   else
     return InsertPartOf(Data, Size, Data, Size, MaxSize);
 }
 
 size_t MutationDispatcher::Mutate_ChangeASCIIInteger(uint8_t *Data, size_t Size,
                                                      size_t MaxSize) {
   if (Size > MaxSize) return 0;

lib/fuzzer/tests/FuzzerUnittest.cpp

 }
 
 TEST(FuzzerMutate, CopyPart1) {
   TestCopyPart(&MutationDispatcher::Mutate_CopyPart, 1 << 10);
 }
 TEST(FuzzerMutate, CopyPart2) {
   TestCopyPart(&MutationDispatcher::Mutate, 1 << 13);
 }
+TEST(FuzzerMutate, CopyPartNoInsertAtMaxSize) {
+  // This (non exhaustively) tests if `Mutate_CopyPart` tries to perform an
+  // insert on an input of size `MaxSize`.  Performing an insert in this case
+  // will lead to the mutation failing.
+  std::unique_ptr<ExternalFunctions> t(new ExternalFunctions());
+  fuzzer::EF = t.get();
+  Random Rand(0);
+  std::unique_ptr<MutationDispatcher> MD(new MutationDispatcher(Rand, {}));
+  uint8_t Data[8] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x00, 0x11, 0x22};
+  size_t MaxSize = sizeof(Data);
+  for (unsigned int count = 0; count < (1 << 18); ++count) {
+    size_t NewSize = MD->Mutate_CopyPart(Data, MaxSize, MaxSize);
+    ASSERT_EQ(NewSize, MaxSize);
+  }
+}
 
 void TestAddWordFromDictionary(Mutator M, int NumIter) {
   std::unique_ptr<ExternalFunctions> t(new ExternalFunctions());
   fuzzer::EF = t.get();
   Random Rand(0);
   std::unique_ptr<MutationDispatcher> MD(new MutationDispatcher(Rand, {}));
   uint8_t Word1[4] = {0xAA, 0xBB, 0xCC, 0xDD};
   uint8_t Word2[3] = {0xFF, 0xEE, 0xEF};