Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick 9b5207569882 from chromium #35544

Merged
merged 2 commits into from Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/chromium/.patches
Expand Up @@ -121,3 +121,4 @@ fix_windows_build_with_enable_plugins_false.patch
remove_default_window_title.patch
add_electron_deps_to_license_credits_file.patch
feat_add_set_can_resize_mutator.patch
cherry-pick-9b5207569882.patch
179 changes: 179 additions & 0 deletions patches/chromium/cherry-pick-9b5207569882.patch
@@ -0,0 +1,179 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ken Rockot <rockot@google.com>
Date: Wed, 31 Aug 2022 15:39:45 +0000
Subject: Mojo: Validate response message type

Ensures that a response message is actually the type expected by the
original request.

Fixed: 1358134
Change-Id: I8f8f58168764477fbf7a6d2e8aeb040f07793d45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3864274
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/heads/main@{#1041553}

diff --git a/mojo/public/cpp/bindings/interface_endpoint_client.h b/mojo/public/cpp/bindings/interface_endpoint_client.h
index df33a20bf5872d5dde43024e3a0722ed4a1d75a3..dcc6e2aa8d9e97ff716d6ab1de02a83eba95eec2 100644
--- a/mojo/public/cpp/bindings/interface_endpoint_client.h
+++ b/mojo/public/cpp/bindings/interface_endpoint_client.h
@@ -221,20 +221,32 @@ class COMPONENT_EXPORT(MOJO_CPP_BINDINGS) InterfaceEndpointClient
void ForgetAsyncRequest(uint64_t request_id);

private:
- // Maps from the id of a response to the MessageReceiver that handles the
- // response.
- using AsyncResponderMap =
- std::map<uint64_t, std::unique_ptr<MessageReceiver>>;
+ struct PendingAsyncResponse {
+ public:
+ PendingAsyncResponse(uint32_t request_message_name,
+ std::unique_ptr<MessageReceiver> responder);
+ PendingAsyncResponse(PendingAsyncResponse&&);
+ PendingAsyncResponse(const PendingAsyncResponse&) = delete;
+ PendingAsyncResponse& operator=(PendingAsyncResponse&&);
+ PendingAsyncResponse& operator=(const PendingAsyncResponse&) = delete;
+ ~PendingAsyncResponse();
+
+ uint32_t request_message_name;
+ std::unique_ptr<MessageReceiver> responder;
+ };
+
+ using AsyncResponderMap = std::map<uint64_t, PendingAsyncResponse>;

struct SyncResponseInfo {
public:
- explicit SyncResponseInfo(bool* in_response_received);
+ SyncResponseInfo(uint32_t request_message_name, bool* in_response_received);

SyncResponseInfo(const SyncResponseInfo&) = delete;
SyncResponseInfo& operator=(const SyncResponseInfo&) = delete;

~SyncResponseInfo();

+ uint32_t request_message_name;
Message response;

// Points to a stack-allocated variable.
diff --git a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc b/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
index b9db8f31a42b956c6125852cf162dc524d5308e6..6e87db197c603d8ac44b591f2cd70023217dcbe2 100644
--- a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
+++ b/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
@@ -28,6 +28,7 @@
#include "mojo/public/cpp/bindings/sync_call_restrictions.h"
#include "mojo/public/cpp/bindings/sync_event_watcher.h"
#include "mojo/public/cpp/bindings/thread_safe_proxy.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/perfetto/protos/perfetto/trace/track_event/chrome_mojo_event_info.pbzero.h"

namespace mojo {
@@ -314,9 +315,27 @@ class ResponderThunk : public MessageReceiverWithStatus {

// ----------------------------------------------------------------------------

+InterfaceEndpointClient::PendingAsyncResponse::PendingAsyncResponse(
+ uint32_t request_message_name,
+ std::unique_ptr<MessageReceiver> responder)
+ : request_message_name(request_message_name),
+ responder(std::move(responder)) {}
+
+InterfaceEndpointClient::PendingAsyncResponse::PendingAsyncResponse(
+ PendingAsyncResponse&&) = default;
+
+InterfaceEndpointClient::PendingAsyncResponse&
+InterfaceEndpointClient::PendingAsyncResponse::operator=(
+ PendingAsyncResponse&&) = default;
+
+InterfaceEndpointClient::PendingAsyncResponse::~PendingAsyncResponse() =
+ default;
+
InterfaceEndpointClient::SyncResponseInfo::SyncResponseInfo(
+ uint32_t request_message_name,
bool* in_response_received)
- : response_received(in_response_received) {}
+ : request_message_name(request_message_name),
+ response_received(in_response_received) {}

InterfaceEndpointClient::SyncResponseInfo::~SyncResponseInfo() {}

@@ -604,6 +623,7 @@ bool InterfaceEndpointClient::SendMessageWithResponder(
// message before calling |SendMessage()| below.
#endif

+ 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);
if (!controller_->SendMessage(message))
@@ -620,7 +640,8 @@ bool InterfaceEndpointClient::SendMessageWithResponder(
controller_->RegisterExternalSyncWaiter(request_id);
}
base::AutoLock lock(async_responders_lock_);
- async_responders_[request_id] = std::move(responder);
+ async_responders_.emplace(
+ request_id, PendingAsyncResponse{message_name, std::move(responder)});
return true;
}

@@ -628,7 +649,8 @@ bool InterfaceEndpointClient::SendMessageWithResponder(

bool response_received = false;
sync_responses_.insert(std::make_pair(
- request_id, std::make_unique<SyncResponseInfo>(&response_received)));
+ request_id,
+ std::make_unique<SyncResponseInfo>(message_name, &response_received)));

base::WeakPtr<InterfaceEndpointClient> weak_self =
weak_ptr_factory_.GetWeakPtr();
@@ -806,13 +828,13 @@ void InterfaceEndpointClient::ResetFromAnotherSequenceUnsafe() {
}

void InterfaceEndpointClient::ForgetAsyncRequest(uint64_t request_id) {
- std::unique_ptr<MessageReceiver> responder;
+ absl::optional<PendingAsyncResponse> response;
{
base::AutoLock lock(async_responders_lock_);
auto it = async_responders_.find(request_id);
if (it == async_responders_.end())
return;
- responder = std::move(it->second);
+ response = std::move(it->second);
async_responders_.erase(it);
}
}
@@ -893,6 +915,10 @@ bool InterfaceEndpointClient::HandleValidatedMessage(Message* message) {
return false;

if (it->second) {
+ if (message->name() != it->second->request_message_name) {
+ return false;
+ }
+
it->second->response = std::move(*message);
*it->second->response_received = true;
return true;
@@ -903,18 +929,22 @@ bool InterfaceEndpointClient::HandleValidatedMessage(Message* message) {
sync_responses_.erase(it);
}

- std::unique_ptr<MessageReceiver> responder;
+ absl::optional<PendingAsyncResponse> pending_response;
{
base::AutoLock lock(async_responders_lock_);
auto it = async_responders_.find(request_id);
if (it == async_responders_.end())
return false;
- responder = std::move(it->second);
+ pending_response = std::move(it->second);
async_responders_.erase(it);
}

+ if (message->name() != pending_response->request_message_name) {
+ return false;
+ }
+
internal::MessageDispatchContext dispatch_context(message);
- return responder->Accept(message);
+ return pending_response->responder->Accept(message);
} else {
if (mojo::internal::ControlMessageHandler::IsControlMessage(message))
return control_message_handler_.Accept(message);