Add an HttpStreamParser fuzzer.

net/http/http_stream_parser_fuzzer.cc

+// 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/http/http_stream_parser.h"
+
+#include <limits>
+#include <string>
+
+#include "base/logging.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/message_loop/message_loop.h"
+#include "net/base/address_list.h"
+#include "net/base/io_buffer.h"
+#include "net/base/net_errors.h"
+#include "net/base/test_completion_callback.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_request_info.h"
+#include "net/http/http_response_info.h"
+#include "net/log/net_log.h"
+#include "net/log/test_net_log.h"
+#include "net/socket/client_socket_handle.h"
+#include "net/socket/socket_test_util.h"
+#include "url/gurl.h"
+
+// Entry point for LibFuzzer.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {

[eroman, 2016/03/25 20:43:18]

I was advised to include:

 #include <stddef.h>
#include <stdint.h>

In past reviews for these.

High chance of them getting included transitively, but may as well for
consistency with other fuzzers.

[mmenke, 2016/03/25 21:33:37]

Done.

+  // Needed for thread checks and waits.
+  base::MessageLoopForIO message_loop;
+
+  // This really shouldn't happen, but just in case...
+  if (size > std::numeric_limits<int>::max())

[eroman, 2016/03/25 20:43:18]

(This certainly will not happen, there is a limit to the sizes used; but good to
be defensive)

[mmenke, 2016/03/25 21:33:37]

No longer relevant (Though if it shouldn't happen, checks like that should be a
CHECK instead)

+    size = std::numeric_limits<int>::max();

[eroman, 2016/03/25 20:43:18]

Since you are using int throughout the rest, I would suggest:
   auto int_size = base::saturated_cast<int>(size);

And then just use int_size later.

[mmenke, 2016/03/25 21:33:37]

I was being paranoid:  2*size can overflow, even if this check passes.  Anyhow,
modified code so it no longer matters.

+
+  const net::IoMode kIoModes[] = {
+      net::ASYNC, net::SYNCHRONOUS,
+  };
+
+  // Try both synchronous and async IO, as they follow slightly different
+  // paths.
+  for (net::IoMode io_mode : kIoModes) {

[eroman, 2016/03/25 20:43:18]

optional nit: auto ?

[mmenke, 2016/03/25 21:33:37]

Done.  I tend to avoid auto except for ugly types, to make code clearer, but
since kIoModes was declared just above, and the name makes it clear, auto seems
fine.

+    net::MockWrite writes[] = {
+        net::MockWrite(net::ASYNC, 0, "GET / HTTP/1.1\r\n\r\n"),
+    };
+
+    // Split the response into three reads of about equal size.  Hopefully this

[eroman, 2016/03/25 20:43:18]

Not sure if you have seen this:

https://code.google.com/p/chromium/codesearch#chromium/src/net/http/http_chun...

It shows a technique for breaking it up into chunk sizes that are a function of
the input, so they are not always the same size.

[mmenke, 2016/03/25 21:33:37]

That doesn't quite work here.  For correct chunk-encoded cases, for instance,
the first four bytes here are "HTTP/", and the last are "0\r\n\r\n" (At least I
think the last chunk has a double CRLF?).  Admittedly, we may not need correct
data to produce read length errors, but that does make me very paranoid.

So instead, I've gone ahead and used the last bytes exclusively as read lengths
+ sync/async indicators.  I do wonder how this sort of silliness can affect the
fuzzer's search patterns.

+    // will allow for catching issues related to multiple body or header reads.
+    int read_offset2 = static_cast<int>(size / 3);
+    int read_offset3 = static_cast<int>(2 * size / 3);
+    net::MockRead reads[] = {
+        net::MockRead(io_mode, reinterpret_cast<const char*>(data),
+                      read_offset2, 1),
+        net::MockRead(io_mode,
+                      reinterpret_cast<const char*>(data) + read_offset2,
+                      read_offset3 - read_offset2, 2),
+        net::MockRead(io_mode,
+                      reinterpret_cast<const char*>(data) + read_offset3,
+                      static_cast<int>(size) - read_offset3, 3),
+        net::MockRead(io_mode, net::ERR_CONNECTION_CLOSED, 4),
+    };
+    net::SequencedSocketData socket_data(reads, arraysize(reads), writes,
+                                         arraysize(writes));
+    socket_data.set_connect_data(net::MockConnect(net::SYNCHRONOUS, net::OK));
+
+    scoped_ptr<net::MockTCPClientSocket> socket(new net::MockTCPClientSocket(
+        net::AddressList(), nullptr, &socket_data));
+
+    net::TestCompletionCallback callback;
+    CHECK_EQ(net::OK, socket->Connect(callback.callback()));
+
+    net::ClientSocketHandle socket_handle;
+    socket_handle.SetSocket(std::move(socket));
+
+    net::HttpRequestInfo request_info;
+    request_info.method = "GET";
+    request_info.url = GURL("http://localhost/");
+
+    scoped_refptr<net::GrowableIOBuffer> read_buffer(
+        new net::GrowableIOBuffer());
+    // Use a NetLog that listens to events, to get coverage of logging
+    // callbacks.
+    net::BoundTestNetLog net_log;
+    net::HttpStreamParser parser(&socket_handle, &request_info,
+                                 read_buffer.get(), net_log.bound());
+
+    net::HttpResponseInfo response_info;
+    int result =
+        parser.SendRequest("GET / HTTP/1.1\r\n", net::HttpRequestHeaders(),
+                           &response_info, callback.callback());
+    CHECK_EQ(net::OK, callback.GetResult(result));
+
+    result = parser.ReadResponseHeaders(callback.callback());
+    result = callback.GetResult(result);
+
+    while (result != net::OK) {

[eroman, 2016/03/25 20:43:18]

Can ReadResponseBody() not return an error in this test?

[mmenke, 2016/03/25 21:33:37]

This is a bug.  This should be while(result > 0).  Yes, GetResult can return an
error.  The bug was actually the success case (net::OK is actually never
returned, even on connection close, I believe, though we should exit the loop if
that happens)

+      scoped_refptr<net::IOBufferWithSize> io_buffer(
+          new net::IOBufferWithSize(size));
+      result = parser.ReadResponseBody(io_buffer.get(), io_buffer->size(),
+                                       callback.callback());
+      result = callback.GetResult(result);
+    }
+  }
+  return 0;
+}