[mojo] Gracefully handle malformed message headers

A Channel message can have some extra header space for stashing e.g.
encoded handle values on Windows, Mac, and Fuchsia. On those platforms
the size is always validated against other header metadata. On other
platforms the size is ignored and never validated.

Meanwhile, when deserializing Channel messages we extract and validate
some basic metadata before allocated a data buffer in which to
reconstruct a sanitized message. On platforms which don't use extra
header bytes, the reconstructed message will always have zero extra
header bytes.

In cases where we receive a malformed message on these platforms, with
some non-zero extra header size, the reconstructed message's total
payload size will differ from the message being deserialized. We have
a DCHECK in place to assert that this never happens, and so such
messages can hit the DCHECK. While this doesn't end up affecting
production behavior in any meaningful way, it does break some
ClusterFuzz test cases.

This updates the deserialization path such that messages with extra
header bytes are rejected on platforms which never use them.

Bug: 968637
Change-Id: I5fa32d3c1f9208472fd899d61f91656b2a710a17
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1638882Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/master@{#665985}

mojo/core/channel.cc

@@ -247,6 +247,11 @@ Channel::MessagePtr Channel::Message::Deserialize(
                 sizeof(MachPortsEntry);
 #else
   const uint32_t max_handles = 0;
+  // No extra header expected. Fail if this is detected.
+  if (extra_header_size > 0) {
+    DLOG(ERROR) << "Decoding invalid message: unexpected extra_header_size > 0";
+    return nullptr;
+  }
 #endif  // defined(OS_WIN)
 
   const uint16_t num_handles =

mojo/core/channel_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/process/process_handle.h"
 #include "base/run_loop.h"
 #include "base/threading/thread.h"
+#include "build/build_config.h"
 #include "mojo/core/platform_handle_utils.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
 #include "testing/gmock/include/gmock/gmock.h"
@@ -364,6 +365,28 @@ TEST(ChannelTest, DeserializeMessage_BadExtraHeaderSize) {
                                                    base::kNullProcessHandle));
 }
 
+#if !defined(OS_WIN) && !defined(OS_MACOSX) && !defined(OS_FUCHSIA)
+TEST(ChannelTest, DeserializeMessage_NonZeroExtraHeaderSize) {
+  // Verifies that a message payload is rejected when the extra header chunk
+  // size anything but zero on Linux, even if it's aligned.
+  constexpr uint16_t kTotalHeaderSize =
+      sizeof(Channel::Message::Header) + kChannelMessageAlignment;
+  constexpr uint32_t kEmptyPayloadSize = 8;
+  constexpr uint32_t kMessageSize = kTotalHeaderSize + kEmptyPayloadSize;
+  char message[kMessageSize];
+  memset(message, 0, kMessageSize);
+
+  Channel::Message::Header* header =
+      reinterpret_cast<Channel::Message::Header*>(&message[0]);
+  header->num_bytes = kMessageSize;
+  header->num_header_bytes = kTotalHeaderSize;
+  header->message_type = Channel::Message::MessageType::NORMAL;
+  header->num_handles = 0;
+  EXPECT_EQ(nullptr, Channel::Message::Deserialize(&message[0], kMessageSize,
+                                                   base::kNullProcessHandle));
+}
+#endif
+
 class CountingChannelDelegate : public Channel::Delegate {
  public:
   explicit CountingChannelDelegate(base::OnceClosure on_final_message)