Add a fuzzer for V8ScriptValueDeserializer.

third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp

Index: third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
diff --git a/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp b/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
new file mode 100644
index 0000000000000000000000000000000000000000..19ae6d2e367def8e9bb60a6b820c6d057f0d8419
--- /dev/null
+++ b/third_party/WebKit/Source/bindings/core/v8/serialization/SerializedScriptValueFuzzer.cpp
@@ -0,0 +1,110 @@
+// 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 "bindings/core/v8/SerializedScriptValue.h"
+
+#include "bindings/core/v8/ScriptState.h"
+#include "core/dom/MessagePort.h"
+#include "core/frame/Settings.h"
+#include "core/testing/DummyPageHolder.h"
+#include "platform/RuntimeEnabledFeatures.h"
+#include "platform/testing/BlinkFuzzerTestSupport.h"
+#include "public/platform/WebBlobInfo.h"
+#include "public/platform/WebMessagePortChannel.h"
+#include "wtf/ByteOrder.h"
+
+#include <algorithm>
+#include <cstddef>
+#include <cstdint>
+#include <v8.h>
+
+using namespace blink;
+
+namespace {
+
+// Intentionally leaked during fuzzing.
+// See testing/libfuzzer/efficient_fuzzer.md.
+DummyPageHolder* pageHolder = nullptr;
+
+enum : uint32_t {
+  kFuzzMessagePorts = 1 << 0,
+  kFuzzBlobInfo = 1 << 1,
+};
+
+class WebMessagePortChannelImpl final : public WebMessagePortChannel {
+ public:
+  // WebMessagePortChannel
+  void setClient(WebMessagePortChannelClient* client) override {}
+  void destroy() override { delete this; }
+  void postMessage(const WebString&, WebMessagePortChannelArray*) {
+    NOTIMPLEMENTED();
+  }
+  bool tryGetMessage(WebString*, WebMessagePortChannelArray&) { return false; }
+};
+
+}  // namespace
+
+extern "C" int LLVMFuzzerInitialize(int* argc, char*** argv) {
+  InitializeBlinkFuzzTest(argc, argv);
+  RuntimeEnabledFeatures::setV8BasedStructuredCloneEnabled(true);
+  pageHolder = DummyPageHolder::create().release();
+  pageHolder->frame().settings()->setScriptEnabled(true);
+  return 0;
+}
+
+extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
+  // Consume a 32-bit fuzzing flags field before the real input.

[mmoroz, 2016/10/07 14:16:38]

Do you really want to consume 32-bit from the data? I think we can use std::hash
(some examples:
https://cs.chromium.org/search/?q=%22std::hash%22+file:.*fuzz.*&sq=package:ch...)
and use data for the deserealization only.

Also in this case we can move the check from line 69 to the beginning of
LLVMFuzzerTestOneInput() function.

[jbroman, 2016/10/07 15:11:14]

It's unintuitive to me why this is better; I would have expected this would
hamper libfuzzer's ability to bias toward unexplored code paths (because part of
the flow control depends on a hash function of the whole input, so how can it
cross-over the choice of flags between inputs?). If it is better (or a
non-issue), then I'm willing to switch to std::hash.

[mmoroz, 2016/10/07 15:44:12]

Yeah, that's a valid concern (I used to prefer this way some time ago too), but
actually:

a) That feels intuitive, but we don't have a real proof that result of the
fuzzing will be biased. And I'm pretty sure it depends on the target.

b) Due to the avalanche effect of a hash function, most likely we will not be
that biased to lose any possible coverage. Assume that we are running fuzzer
without hash (i.e. current patchset) and we have two different inputs with 100%
the same coverage - that's more than real, right? But these two different inputs
will have significantly different hashes (~half of the output bits should
differ), so using hash will allow us to cover different flags combinations
without shrinking the data as well.

c) if we can avoid cutting |data| bytes for flags randomization, we can easily
reuse the corpus, mutate it with another engines or even feed to another fuzzers
(of course if we will have another deserialize() fuzzer :) ).

[jbroman, 2016/10/07 18:27:11]

OK, switched to a hash. Thanks for the explanation.

+  // This lets the fuzzer control whether message ports and blobs are provided.
+  uint32_t flags;
+  if (size < sizeof(flags))
+    return 0;
+  memcpy(&flags, data, sizeof(flags));

[mmoroz, 2016/10/07 14:16:38]

Would you mind to use
https://cs.chromium.org/chromium/src/base/test/fuzzed_data_provider.h class to
consume the data?

No need to use that class if you switch to std::hash for flags randomization.

[jbroman, 2016/10/07 15:11:14]

Ah, wasn't aware of it. base/ is ordinarily banned in core/. Since this is just
a fuzzer it might be okay to ignore that here, but there's also
blink::FuzzedDataProvider (which wraps it, but presently also does an additional
copy of the data -- though it could be adjusted not to).

If you recommend std::hash (above), then I'll use that; otherwise I'll switch to
one of these.

+  data += sizeof(flags);
+  size -= sizeof(flags);
+  flags = ntohl(flags);
+
+  // Odd sizes are handled in various ways, depending how they arrive.
+  // Let's not worry about that case here.
+  if (size % sizeof(UChar))
+    return 0;
+
+  // If message ports are requested, make some.
+  MessagePortArray* messagePorts = nullptr;
+  if (flags & kFuzzMessagePorts) {
+    messagePorts = new MessagePortArray(3);
+    std::generate(messagePorts->begin(), messagePorts->end(), []() {
+      WebMessagePortChannelUniquePtr channel(new WebMessagePortChannelImpl());
+      MessagePort* port = MessagePort::create(pageHolder->document());
+      port->entangle(std::move(channel));
+      return port;
+    });
+  }
+
+  // If blobs are requested, make some.
+  WebBlobInfoArray blobInfoArray;
+  const WebBlobInfoArray* blobs = nullptr;
+  if (flags & kFuzzBlobInfo) {
+    blobInfoArray.emplaceAppend("d875dfc2-4505-461b-98fe-0cf6cc5eaf44",
+                                "text/plain", 12);
+    blobInfoArray.emplaceAppend("d875dfc2-4505-461b-98fe-0cf6cc5eaf44",
+                                "/native/path", "path", "text/plain");
+    blobs = &blobInfoArray;

[mmoroz, 2016/10/07 14:16:38]

Looks like this is read-only:
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/...

If that's correct, does it make sense to move blobInfoArray to a global variable
initialized int he beginning only (i.e. LLVMFuzzerInitialize)?

[jbroman, 2016/10/07 15:11:14]

Done.

+  }
+
+  // Set up.

[haraken, 2016/10/07 02:14:53]

Can you use V8TestingScope?

[jbroman, 2016/10/07 15:11:14]

Since V8TestingScope is STACK_ALLOCATED, this would force me to put it in
LLVMFuzzerTestOneInput, and so the whole DummyPageHolder (and the associated
page, document, frame, clients, etc.) would be instantiated each time. I've
pulled that initialization out so that it only happens once, to speed up fuzzing
time. I haven't measured, but since doing so only costs ~3 LOC, it seemed
worthwhile to extract it.

+  ScriptState* scriptState = ScriptState::forMainWorld(&pageHolder->frame());
+  v8::Isolate* isolate = scriptState->isolate();
+  ScriptState::Scope scope(scriptState);
+  v8::TryCatch tryCatch(isolate);
+
+  // Deserialize.
+  RefPtr<SerializedScriptValue> serializedScriptValue =
+      SerializedScriptValue::create(reinterpret_cast<const char*>(data), size);
+  serializedScriptValue->deserialize(isolate, messagePorts, blobs);
+
+  // Clean up.
+  CHECK(!tryCatch.HasCaught())

[mmoroz, 2016/10/07 14:16:38]

Just to clarify: this is something we don't expect to happen, but would like to
catch if it happens, right?

[jbroman, 2016/10/07 15:11:14]

Right. I'd like to know if an exception somehow escapes
SerializedScriptValue::deserialize.

+      << "deserialize() should return null rather than throwing an exception.";
+  return 0;
+}