Rework the mime sniffer fuzzer.

net/base/mime_sniffer_fuzzer.cc

Index: net/base/mime_sniffer_fuzzer.cc
diff --git a/net/base/mime_sniffer_fuzzer.cc b/net/base/mime_sniffer_fuzzer.cc
new file mode 100644
index 0000000000000000000000000000000000000000..a66ed220fb5f53c946614d5685b797465ae3d19b
--- /dev/null
+++ b/net/base/mime_sniffer_fuzzer.cc
@@ -0,0 +1,56 @@
+// 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/mime_sniffer.h"
+
+#include <stddef.h>
+#include <stdint.h>
+
+#include <string>
+
+#include "base/strings/string_piece.h"
+#include "url/gurl.h"
+
+namespace {
+
+// Finds the line break in |string_piece|, removes every up to and including the

[eroman, 2016/03/28 21:00:25]

This comment is missing something.

removes everything, or removes "every one"

[mmenke, 2016/03/28 21:22:04]

Oops.  Done.  Rewrote this method just a few times.  Too many chefs, and all
that...Even when I'm the only chef.  :)

+// line break from |string_piece|, and returns all the

[eroman, 2016/03/28 21:00:26]

Almost like you ended your thought before -



then the pterodactyl safely landed optimus prime onto the millenium falcon.

[mmenke, 2016/03/28 21:22:04]

That's just silly.  The pterodactyl was a Decepticon, and would never save
Optimus Prime!

+std::string GetNextArgument(base::StringPiece* string_piece) {

[eroman, 2016/03/28 21:00:25]

not a fan of name "string_piece"

|input| would be more descriptive

[mmenke, 2016/03/28 21:22:04]

Done.

+  base::StringPiece::size_type argument_end = string_piece->find('\n');
+  if (argument_end == base::StringPiece::npos)
+    argument_end = string_piece->size();
+  base::StringPiece argument = string_piece->substr(0, argument_end);
+  *string_piece = string_piece->substr(argument_end + 1);
+  return argument.as_string();
+}
+
+}  // namespace
+
+// Fuzzer for the three main mime sniffing functions:
+// ShouldSniffMimeType, SniffMimeType, and SniffMimeTypeFromLocalData.
+//
+// Breaks |data| up into 3 substrings: URL path, MIME type hint, and content,
+// and passes them to all three MIME type functions (Not all functions take all
+// arguments). The first two substrings are each on their own line, and content
+// is everything after them. Since neither URLs nor content-encoding headers can
+// use line breaks, this doesn't reduce coverage.
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+  base::StringPiece input(reinterpret_cast<const char*>(data), size);
+  // Mime sniffing only cares about the path of a URL, so only take the path
+  // part of the URL from the input.
+  std::string path = GetNextArgument(&input);
+  GURL url("https://unfortunate_site_that_relies_on_mime_sniffing/" + path);

[mmenke, 2016/03/28 20:20:59]

This is a bit unfortunate - it means that a lot of the "interesting" search
space the fuzzer discovers will be related to GURL.  :(

We could factor out the code to grab the extension, and update all callsites or
something, but I don't think that's worth the effort.

+
+  std::string mime_type_hint = GetNextArgument(&input);
+
+  net::ShouldSniffMimeType(url, mime_type_hint);

[eroman, 2016/03/28 21:00:26]

This function has a dependence on the URL scheme, however we are only passing in
https:// URLs here.

Would it be better to feed it a URL whose scheme can vary?

[mmenke, 2016/03/28 21:22:04]

I've switched this to take entire URL.

I was taking the path just to reduce the GURL search space (No IDN, no file://
magic, for instance, which has some different rules), as I was horrified how
many of the generated test cases were just probing GURL's logic.  Even with just
the path, we get hundreds of them.  I don't think mime sniffing should ever
depend on scheme in the future - we've pushed back against adding any more
logic, and I think more dependencies on the URL is something we even more
strongly don't want, but that could change, I suppose.

I've also remove the call to ShouldSniffMimeType, and now just check the other
two.  Happy to add it back, it just doesn't seem to do anything exciting enough.

[mmenke, 2016/03/28 21:57:53]

And just to confirm, running it now, 90%+ of the things it saves are it
exploring the file:// and filesystem:// URL spaces, with only one line of data. 
We could force there to be at least two lfs, which would mean as it explores the
URL space, there's a higher chance of it exploring other arguments as well, not
sure it would get us much, though.

It has discovered that .crx is magic, and one of the mime-type hints, at least,
so it's not completely wasting its time.

I find it interesting to see what it comes up with.  Clearly, I need more
excitement in my life.  Maybe I'll take up watching youtube videos of people
bungee jumping.

+
+  std::string result;
+  net::SniffMimeType(input.data(), input.length(), url, mime_type_hint,

[eroman, 2016/03/28 21:00:26]

I presume it may be the case that |!url.is_valid()|. Is this going to hit any
DCHECKs in how |url| is consumed?

[mmenke, 2016/03/28 21:22:04]

So I don't think that case will currently be hit, and the mime type logic
doesn't care, anyways.  Changing URL to be an arbitrary string would cause us to
hit that case, but it still seems to be fine.

+                     &result);
+
+  net::SniffMimeTypeFromLocalData(input.data(), input.length(), &result);
+
+  return 0;
+}