Vary the parameters to WebSocketDeflateStream in fuzzer

net/websockets/websocket_deflate_stream_fuzzer.cc

Index: net/websockets/websocket_deflate_stream_fuzzer.cc
diff --git a/net/websockets/websocket_deflate_stream_fuzzer.cc b/net/websockets/websocket_deflate_stream_fuzzer.cc
index ca401d14d5f0fc91df2890edcb4fc3ac50796cd1..6e133e2abd9497d63cdc09b74f7eb1c9c44d81a1 100644
--- a/net/websockets/websocket_deflate_stream_fuzzer.cc
+++ b/net/websockets/websocket_deflate_stream_fuzzer.cc
@@ -11,6 +11,7 @@
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/test/fuzzed_data_provider.h"
 #include "net/base/completion_callback.h"
@@ -30,14 +31,14 @@ namespace {
 
 class WebSocketFuzzedStream final : public WebSocketStream {
  public:
-  WebSocketFuzzedStream(const uint8_t* data, size_t size)
-      : fuzzed_data_provider_(data, size) {}
+  WebSocketFuzzedStream(base::FuzzedDataProvider* fuzzed_data_provider)
+      : fuzzed_data_provider_(fuzzed_data_provider) {}
 
   int ReadFrames(std::vector<std::unique_ptr<WebSocketFrame>>* frames,
                  const CompletionCallback& callback) override {
-    if (fuzzed_data_provider_.remaining_bytes() == 0)
+    if (fuzzed_data_provider_->remaining_bytes() == 0)
       return ERR_CONNECTION_CLOSED;
-    while (fuzzed_data_provider_.remaining_bytes() > 0)
+    while (fuzzed_data_provider_->remaining_bytes() > 0)
       frames->push_back(CreateFrame());
     return OK;
   }
@@ -54,38 +55,52 @@ class WebSocketFuzzedStream final : public WebSocketStream {
  private:
   std::unique_ptr<WebSocketFrame> CreateFrame() {
     WebSocketFrameHeader::OpCode opcode =
-        fuzzed_data_provider_.ConsumeInt32InRange(
+        fuzzed_data_provider_->ConsumeInt32InRange(
             WebSocketFrameHeader::kOpCodeContinuation,
             WebSocketFrameHeader::kOpCodeControlUnused);
     auto frame = base::MakeUnique<WebSocketFrame>(opcode);
     // Bad news: ConsumeBool actually consumes a whole byte per call, so do
     // something hacky to conserve precious bits.
-    uint8_t flags = fuzzed_data_provider_.ConsumeUint8();
+    uint8_t flags = fuzzed_data_provider_->ConsumeUint8();
     frame->header.final = flags & 0x1;
     frame->header.reserved1 = (flags >> 1) & 0x1;
     frame->header.reserved2 = (flags >> 2) & 0x1;
     frame->header.reserved3 = (flags >> 3) & 0x1;
     frame->header.masked = (flags >> 4) & 0x1;
-    uint64_t payload_length = fuzzed_data_provider_.ConsumeInt32InRange(0, 64);
-    std::string payload = fuzzed_data_provider_.ConsumeBytes(payload_length);
+    uint64_t payload_length = fuzzed_data_provider_->ConsumeInt32InRange(0, 64);
+    std::string payload = fuzzed_data_provider_->ConsumeBytes(payload_length);
     frame->data = new StringIOBuffer(payload);
     frame->header.payload_length = payload.size();
     return frame;
   }
 
-  base::FuzzedDataProvider fuzzed_data_provider_;
+  base::FuzzedDataProvider* fuzzed_data_provider_;

[yhirano, 2017/02/24 12:15:05]

[optional] I'd prefer having this as a unique_ptr, but it's up to you.

[Adam Rice, 2017/02/27 04:00:04]

I'm going to stick with a raw pointer here, so that I can continue to allocate
FuzzedDataProvider on the stack. It's not a big optimisation, given how many
other memory allocations we do, but it's easy to do.

 };
 
 void WebSocketDeflateStreamFuzz(const uint8_t* data, size_t size) {
+  base::FuzzedDataProvider fuzzed_data_provider(data, size);
+  uint8_t flags = fuzzed_data_provider.ConsumeUint8();

[mmoroz, 2017/02/24 12:38:37]

I would like to suggest to consume more bits here, but only if that's possible
that we will have more flags in the future. Assuming that one day we will
consume more than 8 bits here, existing corpus would become mostly useless.

[Adam Rice, 2017/02/27 04:00:04]

Done.

+  bool server_no_context_takeover = flags & 0x1;
+  bool client_no_context_takeover = (flags >> 1) & 0x1;
+  int server_max_window_bits = ((flags >> 2) & 0x7) + 8;
+  int client_max_window_bits = ((flags >> 5) & 0x7) + 8;
   // WebSocketDeflateStream needs to be constructed on each call because it
   // has state.
+  WebSocketExtension params("permessage-deflate");
+  if (server_no_context_takeover)
+    params.Add(WebSocketExtension::Parameter("server_no_context_takeover"));
+  if (client_no_context_takeover)
+    params.Add(WebSocketExtension::Parameter("client_no_context_takeover"));
+  params.Add(WebSocketExtension::Parameter(
+      "server_max_window_bits", base::IntToString(server_max_window_bits)));
+  params.Add(WebSocketExtension::Parameter(
+      "client_max_window_bits", base::IntToString(client_max_window_bits)));
   std::string failure_message;
   WebSocketDeflateParameters parameters;
-  parameters.Initialize(WebSocketExtension("permessage-deflate"),
-                        &failure_message);
+  CHECK(parameters.Initialize(params, &failure_message)) << failure_message;

[yhirano, 2017/02/24 12:15:05]

Is DCHECK enough?

[mmoroz, 2017/02/24 12:38:37]

DCHECK will be enabled only in ASan Debug build. Other configurations are
Release ones. If this CHECK is expected to catch real bugs, this is exactly what
we need.

[Adam Rice, 2017/02/27 04:00:04]

This is a grey area. My conclusion is that we're really only testing for a bug
in the fuzzer here, so DCHECK is appropriate. We have unit tests for whether the
parsing done by WebSocketDeflateParameters works properly.

   WebSocketDeflateStream deflate_stream(
-      base::MakeUnique<WebSocketFuzzedStream>(data, size), parameters,
-      base::MakeUnique<WebSocketDeflatePredictorImpl>());
+      base::MakeUnique<WebSocketFuzzedStream>(&fuzzed_data_provider),
+      parameters, base::MakeUnique<WebSocketDeflatePredictorImpl>());
   std::vector<std::unique_ptr<net::WebSocketFrame>> frames;
   deflate_stream.ReadFrames(&frames, CompletionCallback());
 }