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 #35543

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
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,4 @@ revert_spellcheck_fully_launch_spell_check_delayed_initialization.patch
add_electron_deps_to_license_credits_file.patch
feat_add_set_can_resize_mutator.patch
fix_revert_emulationhandler_update_functions_to_early_return.patch
cherry-pick-9b5207569882.patch
179 changes: 179 additions & 0 deletions patches/chromium/cherry-pick-9b5207569882.patch
Original file line number Diff line number Diff line change
@@ -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 0ebbc94ad51cca74fcebc357b5262229ae5c9d0e..cd79a5edb3f939623b874db36542ee651113c164 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 ded0d20ab193cfdaf36bd44c25719d7073c989fb..a6f41414b8918989662eb0a1dea773932631c8cc 100644
--- a/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
+++ b/mojo/public/cpp/bindings/lib/interface_endpoint_client.cc
@@ -30,6 +30,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 {
@@ -316,9 +317,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() {}

@@ -606,6 +625,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))
@@ -622,7 +642,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;
}

@@ -630,7 +651,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();
@@ -808,13 +830,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);
}
}
@@ -906,6 +928,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;
@@ -916,18 +942,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);