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..efc450f3f2b66f0255848f4a9608a4f131fe9209
--- /dev/null
+++ b/net/base/fuzzed_data_provider.cc
@@ -0,0 +1,53 @@
+// 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 <string>
+
+#include "base/logging.h"
+
+namespace net {
+
+FuzzedDataProvider::FuzzedDataProvider(const uint8_t* data, size_t size)
+    : remaining_data_(reinterpret_cast<const char*>(data), size),
+      unused_bits_(0),
+      num_unused_bits_(0) {}
+
+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);
+  remaining_data_ = remaining_data_.substr(bytes_to_write);
+  return bytes_to_write;
+}
+
+uint32_t FuzzedDataProvider::ConsumeBits(size_t num_bits) {
+  CHECK_NE(0u, num_bits);
+  uint32_t out = 0;
+  while (num_bits > 0) {
+    if (num_unused_bits_ == 0) {
+      unused_bits_ = 0;
+      if (remaining_data_.length() > 0) {

[eroman, 2016/04/22 22:07:10]

nit: !x.empty()

[mmenke, 2016/04/27 19:53:24]

Done.

+        unused_bits_ = remaining_data_.data()[remaining_data_.length() - 1];

[eroman, 2016/04/22 22:07:10]

Why does ConsumeBits() and ConsumeBytes() pull from different ends of the array?
I would have envisioned layering this on top of the ConsumeBytes() primitive
that pulls from the front:

  unused_bits_ = 0;
  num_unused_bits = 8 * ConsumeBytes(&unused_bits, 1);

(Or based on current logic, just num_unused_bits = 8)

[mmenke, 2016/04/25 15:09:18]

For consistency.  Remember that the fuzzer takes previous inputs, and inserts
data into them.

If we have

HTTP/1.1 200 OK
Yada: Yada
YaddaYadda: Yada

Yada Yada Yada
YadaYadaYadaYada
<metadata>

It becomes much easier for the fuzzer to much with the HTTP response while
maintain the full response, than if we have:

<read 8-bytes code>HTTP/1.1 <read 15-bytes code> 200 OK
Yada: Y<read 13-bytes code>ada
...

In the first case, all it needs to do is find a place to add a "SetCookies"
header, for instance, and it's done (Admittedly, the request may start ending in
a read error, or lose a byte or two off the end).  In the second case, it has to
add SetCookies in the right place, and add an exactly matching read length code
as well.

[eroman, 2016/04/25 16:07:12]

OK, if you have run tests and believe this to be a better formulation for input
discovery, then great.

[mmenke, 2016/04/27 19:53:24]

I have now run tests.  I ran each method twice with the URLRequest fuzzer, once
for 20,000 iterations, once for 200,000 iterations.  Admittedly, doesn't really
compare to what the fuzzer servers do.  Both times getting the metadata from the
end did better (~380 vs ~360 on the short run, ~1200 vs ~800 on the longer one).
 No doubt within the margin of error, for such a small sample set, but since the
logic seems to support this approach, at least for how I'm using the fuzzers, I
think this is good enough.

+        remaining_data_ =
+            remaining_data_.substr(0, remaining_data_.length() - 1);
+      }
+      num_unused_bits_ = 8;
+    }
+    size_t bits_to_consume =
+        num_bits <= num_unused_bits_ ? num_bits : num_unused_bits_;

[eroman, 2016/04/22 22:07:10]

Or how about:
bits_to_consume = std::min(num_bits, num_unused_bits);

[mmenke, 2016/04/27 19:53:24]

Now unused is a hard-coded 8, and "std::min(num_bits, 8u);" fails to compile, so
I'm keeping it written out.  static_cast<size_t>(8u) is just weird.

+    uint32_t new_bits = unused_bits_ & ((1 << bits_to_consume) - 1);
+    out = (out << bits_to_consume) + new_bits;

[eroman, 2016/04/22 22:07:10]

Use | instead of +. It will be equivalent in this case, but since these are
bit-level operations I find + confusing.

[mmenke, 2016/04/27 19:53:24]

Done.

+    unused_bits_ >>= bits_to_consume;
+    num_unused_bits_ -= bits_to_consume;
+    num_bits -= bits_to_consume;
+  }
+
+  return out;
+}
+
+}  // namespace net