[Backport] CVE-2021-21198: Out of bounds read in IPC

Manual backport of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2779918:
Don't use BigBuffer for IPC::Message transport

M86 merge conflicts and resolution:
* ipc/ipc_message_pipe_reader.cc
 Fixed extra include.

(cherry picked from commit 85bd7c88523545ab0e497d5e7b3e929793813358)

(cherry picked from commit fad3b9ffe7c7ff82909d911c573bd185aa3b3b50)

Fixed: 1184399
Change-Id: Iddd91ae8d7ae63022b61c96239f5e39261dfb735
Commit-Queue: Ken Rockot <[email protected]>
Reviewed-by: Daniel Cheng <[email protected]>
Cr-Original-Original-Commit-Position: refs/heads/master@{#860010}
Auto-Submit: Ken Rockot <[email protected]>
Reviewed-by: Adrian Taylor <[email protected]>
Reviewed-by: Alex Gough <[email protected]>
Commit-Queue: Alex Gough <[email protected]>
Cr-Original-Commit-Position: refs/branch-heads/4389@{#1597}
Cr-Original-Branched-From: 9251c5db2b6d5a59fe4eac7aafa5fed37c139bb7-refs/heads/master@{#843830}
Reviewed-by: Victor-Gabriel Savu <[email protected]>
Reviewed-by: Artem Sumaneev <[email protected]>
Reviewed-by: Ken Rockot <[email protected]>
Auto-Submit: Artem Sumaneev <[email protected]>
Commit-Queue: Artem Sumaneev <[email protected]>
Cr-Commit-Position: refs/branch-heads/4240@{#1587}
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
Reviewed-by: Jüri Valdmann <[email protected]>

Author: krockot

chromium/ipc/BUILD.gn

@@ -181,10 +181,7 @@ mojom_component("mojom") {
   sources = [
     "ipc.mojom",
   ]
-  public_deps = [
-    "//mojo/public/interfaces/bindings",
-    "//mojo/public/mojom/base",
-  ]
+  public_deps = [ "//mojo/public/interfaces/bindings" ]
 
   # Don't generate a variant sources since we depend on generated internal
   # bindings types and we don't generate or build variants of those.

chromium/ipc/ipc.mojom

@@ -4,7 +4,6 @@
 
 module IPC.mojom;
 
-import "mojo/public/mojom/base/big_buffer.mojom";
 import "mojo/public/interfaces/bindings/native_struct.mojom";
 
 // A placeholder interface type since we don't yet support generic associated
@@ -14,7 +13,7 @@ interface GenericInterface {};
 // Typemapped such that arbitrarily large IPC::Message objects can be sent and
 // received with minimal copying.
 struct Message {
-  mojo_base.mojom.BigBuffer buffer;
+  array<uint8> bytes;
   array<mojo.native.SerializedHandle>? handles;
 };
 
@@ -24,6 +23,7 @@ interface Channel {
   SetPeerPid(int32 pid);
 
   // Transmits a classical Chrome IPC message.
+  [UnlimitedSize]
   Receive(Message message);
 
   // Requests a Channel-associated interface.

chromium/ipc/ipc_message_pipe_reader.cc

@@ -10,6 +10,7 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/containers/span.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
@@ -63,7 +64,9 @@ bool MessagePipeReader::Send(std::unique_ptr<Message> message) {
   if (!sender_)
     return false;
 
-  sender_->Receive(MessageView(*message, std::move(handles)));
+  base::span<const uint8_t> bytes(static_cast<const uint8_t*>(message->data()),
+                                  message->size());
+  sender_->Receive(MessageView(bytes, std::move(handles)));
   DVLOG(4) << "Send " << message->type() << ": " << message->size();
   return true;
 }
@@ -82,11 +85,12 @@ void MessagePipeReader::SetPeerPid(int32_t peer_pid) {
 }
 
 void MessagePipeReader::Receive(MessageView message_view) {
-  if (!message_view.size()) {
+  if (message_view.bytes().empty()) {
     delegate_->OnBrokenDataReceived();
     return;
   }
-  Message message(message_view.data(), message_view.size());
+  Message message(reinterpret_cast<const char*>(message_view.bytes().data()),
+                  message_view.bytes().size());
   if (!message.IsValid()) {
     delegate_->OnBrokenDataReceived();
     return;

chromium/ipc/message.typemap

@@ -11,8 +11,5 @@ sources = [
   "//ipc/message_view.cc",
   "//ipc/message_view.h",
 ]
-public_deps = [
-  "//ipc:message_support",
-  "//mojo/public/cpp/base:shared_typemap_traits",
-]
+public_deps = [ "//ipc:message_support" ]
 type_mappings = [ "IPC.mojom.Message=IPC::MessageView[move_only]" ]

chromium/ipc/message_mojom_traits.cc

@@ -4,15 +4,13 @@
 
 #include "ipc/message_mojom_traits.h"
 
-#include "mojo/public/cpp/base/big_buffer_mojom_traits.h"
-
 namespace mojo {
 
 // static
-mojo_base::BigBufferView
-StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::buffer(
+base::span<const uint8_t>
+StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::bytes(
     IPC::MessageView& view) {
-  return view.TakeBufferView();
+  return view.bytes();
 }
 
 // static
@@ -26,14 +24,14 @@ StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::handles(
 bool StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::Read(
     IPC::mojom::MessageDataView data,
     IPC::MessageView* out) {
-  mojo_base::BigBufferView buffer_view;
-  if (!data.ReadBuffer(&buffer_view))
-    return false;
+  mojo::ArrayDataView<uint8_t> bytes;
+  data.GetBytesDataView(&bytes);
+
   base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles;
   if (!data.ReadHandles(&handles))
     return false;
 
-  *out = IPC::MessageView(std::move(buffer_view), std::move(handles));
+  *out = IPC::MessageView(bytes, std::move(handles));
   return true;
 }
 

chromium/ipc/message_mojom_traits.h

@@ -7,10 +7,10 @@
 
 #include <vector>
 
+#include "base/containers/span.h"
 #include "base/optional.h"
 #include "ipc/ipc.mojom-shared.h"
 #include "ipc/message_view.h"
-#include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/cpp/bindings/struct_traits.h"
 #include "mojo/public/interfaces/bindings/native_struct.mojom.h"
 
@@ -19,7 +19,7 @@ namespace mojo {
 template <>
 class StructTraits<IPC::mojom::MessageDataView, IPC::MessageView> {
  public:
-  static mojo_base::BigBufferView buffer(IPC::MessageView& view);
+  static base::span<const uint8_t> bytes(IPC::MessageView& view);
   static base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles(
       IPC::MessageView& view);
 

chromium/ipc/message_view.cc

@@ -9,17 +9,9 @@ namespace IPC {
 MessageView::MessageView() = default;
 
 MessageView::MessageView(
-    const Message& message,
+    base::span<const uint8_t> bytes,
     base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
-    : buffer_view_(base::make_span<const uint8_t>(
-          static_cast<const uint8_t*>(message.data()),
-          message.size())),
-      handles_(std::move(handles)) {}
-
-MessageView::MessageView(
-    mojo_base::BigBufferView buffer_view,
-    base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
-    : buffer_view_(std::move(buffer_view)), handles_(std::move(handles)) {}
+    : bytes_(bytes), handles_(std::move(handles)) {}
 
 MessageView::MessageView(MessageView&&) = default;
 

chromium/ipc/message_view.h

@@ -11,7 +11,6 @@
 #include "base/containers/span.h"
 #include "base/macros.h"
 #include "ipc/ipc_message.h"
-#include "mojo/public/cpp/base/big_buffer.h"
 #include "mojo/public/interfaces/bindings/native_struct.mojom.h"
 
 namespace IPC {
@@ -20,32 +19,20 @@ class COMPONENT_EXPORT(IPC_MOJOM) MessageView {
  public:
   MessageView();
   MessageView(
-      const Message& message,
-      base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
-  MessageView(
-      mojo_base::BigBufferView buffer_view,
+      base::span<const uint8_t> bytes,
       base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
   MessageView(MessageView&&);
   ~MessageView();
 
   MessageView& operator=(MessageView&&);
 
-  const char* data() const {
-    return reinterpret_cast<const char*>(buffer_view_.data().data());
-  }
-
-  uint32_t size() const {
-    return static_cast<uint32_t>(buffer_view_.data().size());
-  }
-
-  mojo_base::BigBufferView TakeBufferView() { return std::move(buffer_view_); }
-
+  base::span<const uint8_t> bytes() const { return bytes_; }
   base::Optional<std::vector<mojo::native::SerializedHandlePtr>> TakeHandles() {
     return std::move(handles_);
   }
 
  private:
-  mojo_base::BigBufferView buffer_view_;
+  base::span<const uint8_t> bytes_;
   base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles_;
 
   DISALLOW_COPY_AND_ASSIGN(MessageView);