Browse Source

chore: cherry-pick 62142d222a80 from chromium (#33184)

* chore: cherry-pick 62142d222a80 from chromium

* chore: update patches

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Electron Bot <[email protected]>
Jeremy Rose 3 years ago
parent
commit
b6764c0276
2 changed files with 152 additions and 0 deletions
  1. 1 0
      patches/chromium/.patches
  2. 151 0
      patches/chromium/cherry-pick-62142d222a80.patch

+ 1 - 0
patches/chromium/.patches

@@ -150,5 +150,6 @@ cherry-pick-0081bb347e67.patch
 cleanup_pausablecriptexecutor_usage.patch
 cherry-pick-ebc188ad769e.patch
 cherry-pick-1277917.patch
+cherry-pick-62142d222a80.patch
 cherry-pick-1887414c016d.patch
 cherry-pick-6b2643846ae3.patch

+ 151 - 0
patches/chromium/cherry-pick-62142d222a80.patch

@@ -0,0 +1,151 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Ken Rockot <[email protected]>
+Date: Thu, 24 Feb 2022 15:13:13 +0000
+Subject: Validate message headers sooner
+
+M96 merge issues:
+  - multiplex_router.h: conflict in removed lines because of
+  differences in comments above header_validator_
+  - connector.h: conflicting includes
+
+Message header validation has been tied to interface message dispatch,
+but not all mojo::Message consumers are interface bindings.
+
+mojo::Connector is a more general-purpose entry point through which
+incoming messages are received and transformed into mojo::Message
+objects. Blink's MessagePort implementation uses Connector directly to
+transmit and receive raw serialized object data.
+
+This change moves MessageHeaderValidator ownership into Connector and
+always applies its validation immediately after reading a message from
+the pipe, thereby ensuring that all mojo::Message objects used in
+production have validated headers before use.
+
+(cherry picked from commit 8d5bc69146505785ce299c490e35e3f3ef19f69c)
+
+Fixed: 1281908
+Change-Id: Ie0e251ab04681a4fd4b849d82c247e0ed800dc04
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3462461
+Commit-Queue: Ken Rockot <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#971263}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3483815
+Reviewed-by: Victor-Gabriel Savu <[email protected]>
+Owners-Override: Victor-Gabriel Savu <[email protected]>
+Commit-Queue: Roger Felipe Zanoni da Silva <[email protected]>
+Cr-Commit-Position: refs/branch-heads/4664@{#1505}
+Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512}
+
+diff --git a/mojo/public/cpp/bindings/connector.h b/mojo/public/cpp/bindings/connector.h
+index 3975d01a434d6caf229c6d8eaedfe5e2acf684f8..ff17af9b378a1992dbd681e80e957826ebfd83dd 100644
+--- a/mojo/public/cpp/bindings/connector.h
++++ b/mojo/public/cpp/bindings/connector.h
+@@ -18,6 +18,7 @@
+ #include "base/sequenced_task_runner.h"
+ #include "mojo/public/cpp/bindings/connection_group.h"
+ #include "mojo/public/cpp/bindings/message.h"
++#include "mojo/public/cpp/bindings/message_header_validator.h"
+ #include "mojo/public/cpp/bindings/sync_handle_watcher.h"
+ #include "mojo/public/cpp/system/core.h"
+ #include "mojo/public/cpp/system/handle_signal_tracker.h"
+@@ -345,6 +346,8 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) Connector : public MessageReceiver {
+   // The number of pending tasks for |CallDispatchNextMessageFromPipe|.
+   size_t num_pending_dispatch_tasks_ = 0;
+ 
++  MessageHeaderValidator header_validator_;
++
+ #if defined(ENABLE_IPC_FUZZER)
+   std::unique_ptr<MessageReceiver> message_dumper_;
+ #endif
+diff --git a/mojo/public/cpp/bindings/lib/connector.cc b/mojo/public/cpp/bindings/lib/connector.cc
+index 92a9756cbaebe12377e9c9b81467260c73528311..cb08f13b1bfa13a1a79f7f39005f826e51c12a69 100644
+--- a/mojo/public/cpp/bindings/lib/connector.cc
++++ b/mojo/public/cpp/bindings/lib/connector.cc
+@@ -19,6 +19,7 @@
+ #include "base/rand_util.h"
+ #include "base/run_loop.h"
+ #include "base/strings/strcat.h"
++#include "base/strings/string_util.h"
+ #include "base/synchronization/lock.h"
+ #include "base/task/current_thread.h"
+ #include "base/threading/sequence_local_storage_slot.h"
+@@ -152,7 +153,11 @@ Connector::Connector(ScopedMessagePipeHandle message_pipe,
+       force_immediate_dispatch_(!EnableTaskPerMessage()),
+       outgoing_serialization_mode_(g_default_outgoing_serialization_mode),
+       incoming_serialization_mode_(g_default_incoming_serialization_mode),
+-      interface_name_(interface_name) {
++      interface_name_(interface_name),
++      header_validator_(
++          base::JoinString({interface_name ? interface_name : "Generic",
++                            "MessageHeaderValidator"},
++                           "")) {
+   if (config == MULTI_THREADED_SEND)
+     lock_.emplace();
+ 
+@@ -492,6 +497,7 @@ MojoResult Connector::ReadMessage(Message* message) {
+     return result;
+ 
+   *message = Message::CreateFromMessageHandle(&handle);
++
+   if (message->IsNull()) {
+     // Even if the read was successful, the Message may still be null if there
+     // was a problem extracting handles from it. We treat this essentially as
+@@ -507,6 +513,10 @@ MojoResult Connector::ReadMessage(Message* message) {
+     return MOJO_RESULT_ABORTED;
+   }
+ 
++  if (!header_validator_.Accept(message)) {
++    return MOJO_RESULT_ABORTED;
++  }
++
+   return MOJO_RESULT_OK;
+ }
+ 
+diff --git a/mojo/public/cpp/bindings/lib/multiplex_router.cc b/mojo/public/cpp/bindings/lib/multiplex_router.cc
+index 605e51344d64eb5ede3ab475bc8833b37a458535..23e3970089e33d07b18d734d4f599c9c9f9e4bc8 100644
+--- a/mojo/public/cpp/bindings/lib/multiplex_router.cc
++++ b/mojo/public/cpp/bindings/lib/multiplex_router.cc
+@@ -23,7 +23,6 @@
+ #include "mojo/public/cpp/bindings/interface_endpoint_controller.h"
+ #include "mojo/public/cpp/bindings/lib/may_auto_lock.h"
+ #include "mojo/public/cpp/bindings/lib/message_quota_checker.h"
+-#include "mojo/public/cpp/bindings/message_header_validator.h"
+ #include "mojo/public/cpp/bindings/sequence_local_sync_event_watcher.h"
+ 
+ namespace mojo {
+@@ -389,14 +388,7 @@ MultiplexRouter::MultiplexRouter(
+   if (quota_checker)
+     connector_.SetMessageQuotaChecker(std::move(quota_checker));
+ 
+-  std::unique_ptr<MessageHeaderValidator> header_validator =
+-      std::make_unique<MessageHeaderValidator>();
+-  header_validator_ = header_validator.get();
+-  dispatcher_.SetValidator(std::move(header_validator));
+-
+   if (primary_interface_name) {
+-    header_validator_->SetDescription(base::JoinString(
+-        {primary_interface_name, "[primary] MessageHeaderValidator"}, " "));
+     control_message_handler_.SetDescription(base::JoinString(
+         {primary_interface_name, "[primary] PipeControlMessageHandler"}, " "));
+   }
+diff --git a/mojo/public/cpp/bindings/lib/multiplex_router.h b/mojo/public/cpp/bindings/lib/multiplex_router.h
+index 3d3bbb16e25b1adc678e536bd977fa22fc478920..3a9f76a57e301aa89d1bdc5aa1000f815793e238 100644
+--- a/mojo/public/cpp/bindings/lib/multiplex_router.h
++++ b/mojo/public/cpp/bindings/lib/multiplex_router.h
+@@ -38,7 +38,6 @@ class SequencedTaskRunner;
+ namespace mojo {
+ 
+ class AsyncFlusher;
+-class MessageHeaderValidator;
+ class PendingFlush;
+ 
+ namespace internal {
+@@ -301,9 +300,6 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) MultiplexRouter
+ 
+   scoped_refptr<base::SequencedTaskRunner> task_runner_;
+ 
+-  // Owned by |dispatcher_| below.
+-  MessageHeaderValidator* header_validator_ = nullptr;
+-
+   MessageDispatcher dispatcher_;
+   Connector connector_;
+