URLRequest fuzzer.

net/base/fuzzed_data_provider.cc

Index: net/base/fuzzed_data_provider.cc
diff --git a/net/base/fuzzed_data_provider.cc b/net/base/fuzzed_data_provider.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6c940c78dc11bf792ff624b6cb09053c95887d62
--- /dev/null
+++ b/net/base/fuzzed_data_provider.cc
@@ -0,0 +1,67 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "net/base/fuzzed_data_provider.h"
+
+#include <algorithm>
+#include <limits>
+#include <string>
+
+#include "base/logging.h"
+
+namespace net {
+
+FuzzedDataProvider::FuzzedDataProvider(const uint8_t* data, size_t size)
+    : remaining_data_(reinterpret_cast<const char*>(data), size) {}
+
+FuzzedDataProvider::~FuzzedDataProvider() {}
+
+size_t FuzzedDataProvider::ConsumeBytes(char* dest, size_t bytes) {
+  size_t bytes_to_write = std::min(bytes, remaining_data_.length());
+  memcpy(dest, remaining_data_.data(), bytes_to_write);

[eroman, 2016/04/28 01:32:35]

side-rant: unfortunately because the definition of memcpy() is complete rubbist,
this can lead to undefined behavior if ever remaining_data_ was empty (in such a
way that remaining_data_.data() is NULL.

Because naturally, memcpy(X, NULL, 0) is undefined behavior. Which is a
completely insane of course, but yay C? (for instance see
https://gcc.gnu.org/gcc-4.9/porting_to.html and the inferences it makes from
memmove as an example)

I hate memcpy() anyway -- probably just make ConsumeBytes() return a string
piece and let the consumer worry about copying?

[mmenke, 2016/04/28 16:01:38]

remaining_data_.data() cannot be NULL, I believe.  It can only be of length 0.

Anyhow, I've swtiched to returning a StringPiece, and now use std::copy in
FuzzedSocket

+  remaining_data_ = remaining_data_.substr(bytes_to_write);
+  return bytes_to_write;
+}
+
+uint32_t FuzzedDataProvider::ConsumeValueInRange(uint32_t min, uint32_t max) {
+  CHECK_LE(min, max);
+
+  uint32_t range = max - min;
+  uint32_t offset = 0;
+  uint32_t result = 0;
+
+  while ((range >> offset) > 0 && !remaining_data_.empty()) {

[eroman, 2016/04/28 01:32:35]

much simpler than earlier impl!

+    // Pull bytes off the end of the seed data. Experimentally, this seems to
+    // allow the fuzzer to more easily explore the input space. This makes
+    // sense, since it works by modifying inputs that caused new code to run,
+    // and this data is often used to encode length of data read by
+    // ConsumeBytes. Separating out read lengths makes it easier modify the
+    // contents of the data that is actually read.
+    uint8_t next_byte = remaining_data_.data()[remaining_data_.length() - 1];
+    result = (result << 8) | next_byte;
+    remaining_data_ = remaining_data_.substr(0, remaining_data_.length() - 1);
+    offset += 8;
+  }
+
+  // Avoid division by 0, in the case the entire range is covered.
+  if (min == 0 && max == std::numeric_limits<uint32_t>::max())

[eroman, 2016/04/28 01:32:36]

nit: sufficient to test range == std::numeric_limits<uint32_t>::max()

A little bit clearer anyway, since the division by 0 comes from overflow on
(range + 1).

[mmenke, 2016/04/28 16:01:38]

Done.

+    return result;
+
+  return min + result % (range + 1);
+}
+
+uint32_t FuzzedDataProvider::ConsumeBits(size_t num_bits) {
+  CHECK_NE(0u, num_bits);
+  CHECK_LE(num_bits, 32u);
+
+  if (num_bits == 32)
+    return ConsumeValueInRange(0, std::numeric_limits<uint32_t>::max());
+  return ConsumeValueInRange(0, (1 << num_bits) - 1);
+}
+
+bool FuzzedDataProvider::ConsumeBool() {
+  return !ConsumeBits(1);

[eroman, 2016/04/28 01:32:36]

So before in some places you were using !!ConsumeBits(1).

Conceptually doesn't matter, except for what you want the default to be in the
case where there is no data left (default true with this impl).

[mmenke, 2016/04/28 16:01:38]

Ahh, good point.  Changed, and added comment.

+}
+
+}  // namespace net