[Backport] CVE-2022-4178: Use after free in Mojo (1/2)

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3989408:
Mojo: Disable sync call interrupts in the browser

M102 merge issues:
  content/app/content_main_runner_impl.cc:
    should_start_minimal_browser is present in 102 but not on main
  mojo/public/cpp/bindings/lib/sync_call_restrictions.cc:
    include conflicts, base/check_op.h isn't included in 102

This changes the default Mojo sync call behavior in the browser process
to prevent any blocking sync calls from being interrupted by other
incoming sync IPC dispatches.

Bug: 1376099
Change-Id: I53681ef379fdd3c2bfc37d7e16b3de17acad5d20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3989408
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1065369}
(cherry picked from commit b6f921260e0e763db7a72de9c7a3f0f78a99f21f)
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/446482
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

6 files changed, 141 insertions, 20 deletions

diff --git a/chromium/content/app/content_main_runner_impl.cc b/chromium/content/app/content_main_runner_impl.cc
index c7c49c07018..395aebc0ed5 100644
--- a/chromium/content/app/content_main_runner_impl.cc
+++ b/chromium/content/app/content_main_runner_impl.cc
@@ -92,6 +92,7 @@
 #include "media/media_buildflags.h"
 #include "mojo/core/embedder/embedder.h"
 #include "mojo/public/cpp/bindings/self_owned_receiver.h"
+#include "mojo/public/cpp/bindings/sync_call_restrictions.h"
 #include "mojo/public/cpp/platform/platform_channel.h"
 #include "mojo/public/cpp/system/dynamic_library_support.h"
 #include "mojo/public/cpp/system/invitation.h"
@@ -1039,7 +1040,13 @@ int ContentMainRunnerImpl::RunBrowser(MainFunctionParams main_params,
   if (is_browser_main_loop_started_)
     return -1;
 
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSingleProcess)) {
+    mojo::SyncCallRestrictions::DisableSyncCallInterrupts();
+  }
+
   bool should_start_minimal_browser = start_minimal_browser;
+
   if (!mojo_ipc_support_) {
     if (delegate_->ShouldCreateFeatureList()) {
       // This is intentionally leaked since it needs to live for the duration
diff --git a/chromium/ipc/ipc_mojo_bootstrap.cc b/chromium/ipc/ipc_mojo_bootstrap.cc
index a0bd86951a2..f35570d2450 100644
--- a/chromium/ipc/ipc_mojo_bootstrap.cc
+++ b/chromium/ipc/ipc_mojo_bootstrap.cc
@@ -16,13 +16,15 @@
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/check_op.h"
+#include "base/containers/circular_deque.h"
 #include "base/containers/contains.h"
-#include "base/containers/queue.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/no_destructor.h"
+#include "base/ranges/algorithm.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/lock.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task/common/task_annotator.h"
 #include "base/task/sequenced_task_runner.h"
 #include "base/task/single_thread_task_runner.h"
@@ -48,6 +50,7 @@
 #include "mojo/public/cpp/bindings/pipe_control_message_proxy.h"
 #include "mojo/public/cpp/bindings/sequence_local_sync_event_watcher.h"
 #include "mojo/public/cpp/bindings/tracing_helpers.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace IPC {
 
@@ -466,6 +469,11 @@ class ChannelAssociatedGroupController
       return *this;
     }
 
+    bool HasRequestId(uint64_t request_id) {
+      return !value_.IsNull() && value_.version() >= 1 &&
+             value_.header_v1()->request_id == request_id;
+    }
+
     mojo::Message& value() { return value_; }
 
    private:
@@ -557,10 +565,15 @@ class ChannelAssociatedGroupController
       sync_watcher_.reset();
     }
 
-    uint32_t EnqueueSyncMessage(MessageWrapper message) {
+    absl::optional<uint32_t> EnqueueSyncMessage(MessageWrapper message) {
       controller_->lock_.AssertAcquired();
+      if (exclusive_wait_ && exclusive_wait_->TryFulfillingWith(message)) {
+        exclusive_wait_ = nullptr;
+        return absl::nullopt;
+      }
+
       uint32_t id = GenerateSyncMessageId();
-      sync_messages_.emplace(id, std::move(message));
+      sync_messages_.emplace_back(id, std::move(message));
       SignalSyncMessageEvent();
       return id;
     }
@@ -577,7 +590,7 @@ class ChannelAssociatedGroupController
       if (sync_messages_.empty() || sync_messages_.front().first != id)
         return MessageWrapper();
       MessageWrapper message = std::move(sync_messages_.front().second);
-      sync_messages_.pop();
+      sync_messages_.pop_front();
       return message;
     }
 
@@ -607,10 +620,38 @@ class ChannelAssociatedGroupController
       return sync_watcher_->SyncWatch(&should_stop);
     }
 
+    MessageWrapper WaitForIncomingSyncReply(uint64_t request_id) {
+      absl::optional<ExclusiveSyncWait> wait;
+      {
+        base::AutoLock lock(controller_->lock_);
+        for (auto& [id, message] : sync_messages_) {
+          if (message.HasRequestId(request_id)) {
+            return std::move(message);
+          }
+        }
+
+        DCHECK(!exclusive_wait_);
+        wait.emplace(request_id);
+        exclusive_wait_ = &wait.value();
+      }
+
+      wait->event.Wait();
+      return std::move(wait->message);
+    }
+
     bool SyncWatchExclusive(uint64_t request_id) override {
-      // We don't support exclusive waits on Channel-associated interfaces.
-      NOTREACHED();
-      return false;
+      MessageWrapper message = WaitForIncomingSyncReply(request_id);
+      if (message.value().IsNull() || !client_) {
+        return false;
+      }
+
+      if (!client_->HandleIncomingMessage(&message.value())) {
+        base::AutoLock locker(controller_->lock_);
+        controller_->RaiseError();
+        return false;
+      }
+
+      return true;
     }
 
     void RegisterExternalSyncWaiter(uint64_t request_id) override {}
@@ -624,6 +665,9 @@ class ChannelAssociatedGroupController
       DCHECK(closed_);
       DCHECK(peer_closed_);
       DCHECK(!sync_watcher_);
+      if (exclusive_wait_) {
+        exclusive_wait_->event.Signal();
+      }
     }
 
     void OnSyncMessageEventReady() {
@@ -637,7 +681,7 @@ class ChannelAssociatedGroupController
       if (!sync_messages_.empty()) {
         MessageWrapper message_wrapper =
             std::move(sync_messages_.front().second);
-        sync_messages_.pop();
+        sync_messages_.pop_front();
 
         bool dispatch_succeeded;
         mojo::InterfaceEndpointClient* client = client_;
@@ -685,6 +729,28 @@ class ChannelAssociatedGroupController
       return id;
     }
 
+    // Tracks the state of a pending sync wait which excludes all other incoming
+    // IPC on the waiting thread.
+    struct ExclusiveSyncWait {
+      explicit ExclusiveSyncWait(uint64_t request_id)
+          : request_id(request_id) {}
+      ~ExclusiveSyncWait() = default;
+
+      bool TryFulfillingWith(MessageWrapper& wrapper) {
+        if (!wrapper.HasRequestId(request_id)) {
+          return false;
+        }
+
+        message = std::move(wrapper);
+        event.Signal();
+        return true;
+      }
+
+      uint64_t request_id;
+      base::WaitableEvent event;
+      MessageWrapper message;
+    };
+
     const raw_ptr<ChannelAssociatedGroupController> controller_;
     const mojo::InterfaceId id_;
 
@@ -696,7 +762,8 @@ class ChannelAssociatedGroupController
     raw_ptr<mojo::InterfaceEndpointClient> client_ = nullptr;
     scoped_refptr<base::SequencedTaskRunner> task_runner_;
     std::unique_ptr<mojo::SequenceLocalSyncEventWatcher> sync_watcher_;
-    base::queue<std::pair<uint32_t, MessageWrapper>> sync_messages_;
+    base::circular_deque<std::pair<uint32_t, MessageWrapper>> sync_messages_;
+    ExclusiveSyncWait* exclusive_wait_ = nullptr;
     uint32_t next_sync_message_id_ = 0;
   };
 
@@ -929,12 +996,15 @@ class ChannelAssociatedGroupController
         // sync message queue. If the endpoint was blocking, it will dequeue the
         // message and dispatch it. Otherwise the posted |AcceptSyncMessage()|
         // call will dequeue the message and dispatch it.
-        uint32_t message_id =
+        absl::optional<uint32_t> message_id =
             endpoint->EnqueueSyncMessage(std::move(message_wrapper));
-        task_runner->PostTask(
-            FROM_HERE,
-            base::BindOnce(&ChannelAssociatedGroupController::AcceptSyncMessage,
-                           this, id, message_id));
+        if (message_id) {
+          task_runner->PostTask(
+              FROM_HERE,
+              base::BindOnce(
+                  &ChannelAssociatedGroupController::AcceptSyncMessage, this,
+                  id, *message_id));
+        }
         return true;
       }
 
diff --git a/chromium/mojo/public/cpp/bindings/interface_endpoint_controller.h b/chromium/mojo/public/cpp/bindings/interface_endpoint_controller.h
index 89dbe399946..8649abe1ac9 100644
--- a/chromium/mojo/public/cpp/bindings/interface_endpoint_controller.h
+++ b/chromium/mojo/public/cpp/bindings/interface_endpoint_controller.h
@@ -36,6 +36,10 @@ class InterfaceEndpointController {
   // Watches the endpoint for a specific incoming sync reply. This method only
   // returns true once the reply is received, or false if the endpoint is
   // detached or destroyed beforehand.
+  //
+  // Unlike with SyncWatch(), no other IPCs (not even other sync IPCs) can be
+  // dispatched to the calling thread while SyncWatchExclusive() is waiting on
+  // the reply for `request_id`.
   virtual bool SyncWatchExclusive(uint64_t request_id) = 0;
 
   // Notifies the controller that a specific in-flight sync message identified
diff --git a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
index 6e87db197c6..1a786923f6d 100644
--- a/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
+++ b/chromium/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
@@ -387,7 +387,9 @@ void ThreadSafeInterfaceEndpointClientProxy::SendMessageWithResponder(
   }
 
   // If the Remote is bound on another sequence, post the call.
-  const bool allow_interrupt = !message.has_flag(Message::kFlagNoInterrupt);
+  const bool allow_interrupt =
+      SyncCallRestrictions::AreSyncCallInterruptsEnabled() &&
+      !message.has_flag(Message::kFlagNoInterrupt);
   auto response = base::MakeRefCounted<SyncResponseInfo>();
   auto response_signaler = std::make_unique<SyncResponseSignaler>(response);
   task_runner_->PostTask(
@@ -625,7 +627,9 @@ bool InterfaceEndpointClient::SendMessageWithResponder(
 
   const uint32_t message_name = message->name();
   const bool is_sync = message->has_flag(Message::kFlagIsSync);
-  const bool exclusive_wait = message->has_flag(Message::kFlagNoInterrupt);
+  const bool exclusive_wait =
+      message->has_flag(Message::kFlagNoInterrupt) ||
+      !SyncCallRestrictions::AreSyncCallInterruptsEnabled();
   if (!controller_->SendMessage(message))
     return false;
 
diff --git a/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc b/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc
index fa7934bdb34..166b9bdc505 100644
--- a/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc
+++ b/chromium/mojo/public/cpp/bindings/lib/sync_call_restrictions.cc
@@ -4,8 +4,6 @@
 
 #include "mojo/public/cpp/bindings/sync_call_restrictions.h"
 
-#if ENABLE_SYNC_CALL_RESTRICTIONS
-
 #include "base/debug/leak_annotations.h"
 #include "base/logging.h"
 #include "base/no_destructor.h"
@@ -18,6 +16,11 @@ namespace mojo {
 
 namespace {
 
+// Sync call interrupts are enabled by default.
+bool g_enable_sync_call_interrupts = true;
+
+#if ENABLE_SYNC_CALL_RESTRICTIONS
+
 class GlobalSyncCallSettings {
  public:
   GlobalSyncCallSettings() = default;
@@ -60,8 +63,12 @@ bool SyncCallRestrictionsEnforceable() {
   return base::internal::SequenceLocalStorageMap::IsSetForCurrentThread();
 }
 
+#endif  // ENABLE_SYNC_CALL_RESTRICTIONS
+
 }  // namespace
 
+#if ENABLE_SYNC_CALL_RESTRICTIONS
+
 // static
 void SyncCallRestrictions::AssertSyncCallAllowed() {
   if (GetGlobalSettings().sync_call_allowed_by_default() ||
@@ -101,6 +108,21 @@ void SyncCallRestrictions::DecreaseScopedAllowCount() {
   --GetSequenceLocalScopedAllowCount();
 }
 
-}  // namespace mojo
-
 #endif  // ENABLE_SYNC_CALL_RESTRICTIONS
+
+// static
+void SyncCallRestrictions::DisableSyncCallInterrupts() {
+  g_enable_sync_call_interrupts = false;
+}
+
+// static
+void SyncCallRestrictions::EnableSyncCallInterruptsForTesting() {
+  g_enable_sync_call_interrupts = true;
+}
+
+// static
+bool SyncCallRestrictions::AreSyncCallInterruptsEnabled() {
+  return g_enable_sync_call_interrupts;
+}
+
+}  // namespace mojo
diff --git a/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h b/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h
index e7e67ee824b..1653fd63033 100644
--- a/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h
+++ b/chromium/mojo/public/cpp/bindings/sync_call_restrictions.h
@@ -86,6 +86,20 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) SyncCallRestrictions {
   static void DisallowSyncCall() {}
 #endif
 
+  // Globally disables sync call interrupts. This means that all sync calls in
+  // the current process will be strictly blocking until a reply is received,
+  // and no incoming sync calls can dispatch on the blocking thread in interim.
+  static void DisableSyncCallInterrupts();
+
+  // Used only in tests to re-enable sync call interrupts after disabling them.
+  static void EnableSyncCallInterruptsForTesting();
+
+  // Indicates whether sync call interrupts are enabled in the calling process.
+  // They're enabled by default, so any sync message that isn't marked [Sync]
+  // may have its blocking call interrupted to dispatch other incoming sync
+  // IPCs which target the blocking thread.
+  static bool AreSyncCallInterruptsEnabled();
+
  private:
   // DO NOT ADD ANY OTHER FRIEND STATEMENTS, talk to mojo/OWNERS first.
   // BEGIN ALLOWED USAGE.