libFuzzer for blink::MHTMLParser

third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp

Index: third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp
diff --git a/third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp b/third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..d8cc3c3b307f32d87cfc5a442feff02f26fc2928
--- /dev/null
+++ b/third_party/WebKit/Source/platform/mhtml/MHTMLFuzzer.cpp
@@ -0,0 +1,61 @@
+// 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 "platform/SharedBuffer.h"
+#include "platform/heap/Handle.h"
+#include "platform/mhtml/ArchiveResource.h"
+#include "platform/mhtml/MHTMLParser.h"
+#include "platform/testing/TestingPlatformSupport.h"
+#include "wtf/Assertions.h"
+#include "wtf/Compiler.h"
+
+#include <base/command_line.h>
+#include <base/i18n/icu_util.h>
+
+#include <memory>
+
+#include <stddef.h>
+#include <stdint.h>
+
+
+namespace blink {
+
+// Fuzzer for blink::MHTMLParser.
+int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
+{
+    MHTMLParser mhtmlParser(SharedBuffer::create(data, size));
+    HeapVector<Member<ArchiveResource>> mhtmlArchives = mhtmlParser.parseArchive();
+    for (const auto& mhtmlArchive : mhtmlArchives) {
+        // TODO(lukasza): Should MHTMLParser guarantee returning valid data?
+        // ASSERT(mhtmlArchive->url().isValid());

[mmoroz, 2016/08/01 17:41:37]

It would be better to avoid asserts in the target function. I mean, if it is
something what we may hit by passing invalid inputs (LibFuzzer obviously does
that), we don't want to hit asserts, return 0 is needed. If assert is for
something impossible (i.e. real problem which we actually do not expect to
happen), it may take place.

[Łukasz Anforowicz, 2016/08/01 21:09:39]

I was wondering if for an invalid input, MHTMLParser should return an empty
vector (rather than returning a vector with invalid data inside).  I am not sure
if this is desirable - you're probably right that the current API contract is
sufficient.

+        // ASSERT(!mhtmlArchive->mimeType().isEmpty());
+        ALLOW_UNUSED_LOCAL(mhtmlArchive);
+    }
+
+    // Skipping garbage collection on purpose - it doubles exec/s of libfuzzer runs.
+    // This probably needs to be uncommented to support -detect_leaks=1 flag.

[mmoroz, 2016/08/01 17:41:37]

Good point. If LSan detects that, we have to enable GC. Otherwise we will be
always crashing on leaks :(

[Łukasz Anforowicz, 2016/08/01 21:09:39]

Ok - done.

Also - there is no perf regression even with GC.  I guess there were some
external variables that influenced my initial libfuzzer runs (right now I
consistently get 800+ exec/s with and without the collectAllGarbage call).

+    // mhtmlArchives.clear();
+    // ThreadHeap::collectAllGarbage();
+
+    return 0;
+}
+
+} // namespace blink
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
+{
+    return blink::LLVMFuzzerTestOneInput(data, size);
+}
+
+extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv)
+{
+    base::CommandLine::Init(*argc, *argv);
+    base::i18n::InitializeICU();
+
+    // Intentional leak - no need to do cleanup as explained in
+    // "Initialization/Cleanup" section of testing/libfuzzer/efficient_fuzzer.md
+    new blink::ScopedUnittestsEnvironmentSetup();

[Łukasz Anforowicz, 2016/08/01 21:09:39]

Ooops.  This results in a leak report by libfuzzer.  I need to change this to a
static variable instead.

+
+    return 0;
+}