Vary the parameters to WebSocketDeflateStream in fuzzer

net/websockets/websocket_deflate_stream_fuzzer.cc

 // Copyright 2015 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 <stddef.h>
 #include <stdint.h>
 
 #include <memory>
 #include <string>
 #include <vector>
 
 #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"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/websockets/websocket_deflate_parameters.h"
 #include "net/websockets/websocket_deflate_predictor.h"
 #include "net/websockets/websocket_deflate_predictor_impl.h"
 #include "net/websockets/websocket_deflate_stream.h"
 #include "net/websockets/websocket_extension.h"
 #include "net/websockets/websocket_frame.h"
 #include "net/websockets/websocket_stream.h"
 
 namespace net {
 
 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;
   }
 
   int WriteFrames(std::vector<std::unique_ptr<WebSocketFrame>>* frames,
                   const CompletionCallback& callback) override {
     return ERR_FILE_NOT_FOUND;
   }
 
   void Close() override {}
   std::string GetSubProtocol() const override { return std::string(); }
   std::string GetExtensions() const override { return std::string(); }
 
  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());
 }
 
 }  // namespace
 
 }  // namespace net
 
 // Entry point for LibFuzzer.
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {

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

It might be useful to add something like |if (size < N) return 0;| in the
beginning of the fuzz target, where N is minimum amount of bytes we need to
generate parameters. Looks like N is 10 here. That would let libFuzzer to know
that inputs shorter than 10 bytes are useless.

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

That's a good tip, thanks. Because ConsumeUint32InRange() only consumes 1 byte
for the ranges we use, I ended up with N = 4.

   net::WebSocketDeflateStreamFuzz(data, size);
 
   return 0;
 }