summaryrefslogtreecommitdiffstats
path: root/chromium
diff options
context:
space:
mode:
authorKen Rockot <rockot@google.com>2022-02-24 15:13:13 +0000
committerMichael BrĂ¼ning <michael.bruning@qt.io>2022-05-19 14:55:34 +0000
commit2ceec49c31b00b5f9b609d69ecf88250f8a020ac (patch)
tree83cb72044465b2c7c16e373e7c749c8106e8ca2b /chromium
parent0d984c7f044a08975088191f92ecc9bc62424b14 (diff)
[Backport] CVE-2022-0797: Out of bounds memory access in Mojo
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/3483815: [M96-LTS][mojo-bindings]: 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 Commit-Queue: Ken Rockot <rockot@google.com> Cr-Original-Commit-Position: refs/heads/main@{#971263} Reviewed-by: Victor-Gabriel Savu <vsavu@google.com> Owners-Override: Victor-Gabriel Savu <vsavu@google.com> Commit-Queue: Roger Felipe Zanoni da Silva <rzanoni@google.com> Cr-Commit-Position: refs/branch-heads/4664@{#1505} Cr-Branched-From: 24dc4ee75e01a29d390d43c9c264372a169273a7-refs/heads/main@{#929512} Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Diffstat (limited to 'chromium')
-rw-r--r--chromium/mojo/public/cpp/bindings/connector.h3
-rw-r--r--chromium/mojo/public/cpp/bindings/lib/connector.cc12
-rw-r--r--chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc7
-rw-r--r--chromium/mojo/public/cpp/bindings/lib/multiplex_router.h4
4 files changed, 14 insertions, 12 deletions
diff --git a/chromium/mojo/public/cpp/bindings/connector.h b/chromium/mojo/public/cpp/bindings/connector.h
index 1616f1bf023..b5c89c9fa85 100644
--- a/chromium/mojo/public/cpp/bindings/connector.h
+++ b/chromium/mojo/public/cpp/bindings/connector.h
@@ -19,6 +19,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"
@@ -319,6 +320,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/chromium/mojo/public/cpp/bindings/lib/connector.cc b/chromium/mojo/public/cpp/bindings/lib/connector.cc
index 93ea25007ec..bdd91fc89df 100644
--- a/chromium/mojo/public/cpp/bindings/lib/connector.cc
+++ b/chromium/mojo/public/cpp/bindings/lib/connector.cc
@@ -17,6 +17,7 @@
#include "base/no_destructor.h"
#include "base/rand_util.h"
#include "base/run_loop.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"
@@ -153,7 +154,11 @@ Connector::Connector(ScopedMessagePipeHandle message_pipe,
outgoing_serialization_mode_(g_default_outgoing_serialization_mode),
incoming_serialization_mode_(g_default_incoming_serialization_mode),
heap_profiler_tag_(heap_profiler_tag),
- nesting_observer_(RunLoopNestingObserver::GetForThread()) {
+ nesting_observer_(RunLoopNestingObserver::GetForThread()),
+ header_validator_(
+ base::JoinString({heap_profiler_tag ? heap_profiler_tag : "Generic",
+ "MessageHeaderValidator"},
+ "")) {
if (config == MULTI_THREADED_SEND)
lock_.emplace();
@@ -454,6 +459,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
@@ -468,6 +474,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/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc
index 5f2e8141fe1..fa04dc6c680 100644
--- a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc
+++ b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.cc
@@ -351,15 +351,8 @@ 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) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
- header_validator_->SetDescription(base::JoinString(
- {primary_interface_name, "[primary] MessageHeaderValidator"}, " "));
control_message_handler_.SetDescription(base::JoinString(
{primary_interface_name, "[primary] PipeControlMessageHandler"}, " "));
}
diff --git a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h
index 49c6785ca05..a63937c7185 100644
--- a/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h
+++ b/chromium/mojo/public/cpp/bindings/lib/multiplex_router.h
@@ -29,7 +29,6 @@
#include "mojo/public/cpp/bindings/connector.h"
#include "mojo/public/cpp/bindings/interface_id.h"
#include "mojo/public/cpp/bindings/message_dispatcher.h"
-#include "mojo/public/cpp/bindings/message_header_validator.h"
#include "mojo/public/cpp/bindings/pending_flush.h"
#include "mojo/public/cpp/bindings/pipe_control_message_handler.h"
#include "mojo/public/cpp/bindings/pipe_control_message_handler_delegate.h"
@@ -259,9 +258,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_;