Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
chore: cherry-pick 1536a564d959 from chromium (#28812)
* chore: cherry-pick 1536a564d959 from chromium * update patches Co-authored-by: Electron Bot <electron@github.com> Co-authored-by: Cheng Zhao <zcbenz@gmail.com>
- Loading branch information
1 parent
37d42ec
commit 33036aa
Showing
2 changed files
with
100 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,99 @@ | ||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 | ||
From: Ken Rockot <rockot@google.com> | ||
Date: Thu, 15 Apr 2021 18:14:57 +0000 | ||
Subject: Mojo: Properly validate broadcast events | ||
|
||
This corrects broadcast event deserialization by adding a missing | ||
validation step when decoding the outer message header. | ||
|
||
(cherry picked from commit 6740adb28374ddeee13febfd5e5d20cb8a365979) | ||
|
||
Fixed: 1195308 | ||
Change-Id: Ia67a20e48614e7ef00b1b32f7f4e5f20235be310 | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2808678 | ||
Reviewed-by: Daniel Cheng <dcheng@chromium.org> | ||
Commit-Queue: Ken Rockot <rockot@google.com> | ||
Cr-Original-Commit-Position: refs/heads/master@{#870238} | ||
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2827760 | ||
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> | ||
Auto-Submit: Ken Rockot <rockot@google.com> | ||
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> | ||
Cr-Commit-Position: refs/branch-heads/4430@{#1290} | ||
Cr-Branched-From: e5ce7dc4f7518237b3d9bb93cccca35d25216cbe-refs/heads/master@{#857950} | ||
|
||
diff --git a/mojo/core/node_channel.cc b/mojo/core/node_channel.cc | ||
index c48fb573fea961aa4125c25f065a5177e26b7732..31da822da87abbfa0e2fb416e62e8009d028bde1 100644 | ||
--- a/mojo/core/node_channel.cc | ||
+++ b/mojo/core/node_channel.cc | ||
@@ -251,13 +251,16 @@ Channel::MessagePtr NodeChannel::CreateEventMessage(size_t capacity, | ||
} | ||
|
||
// static | ||
-void NodeChannel::GetEventMessageData(Channel::Message* message, | ||
+bool NodeChannel::GetEventMessageData(Channel::Message& message, | ||
void** data, | ||
size_t* num_data_bytes) { | ||
- // NOTE: OnChannelMessage guarantees that we never accept a Channel::Message | ||
- // with a payload of fewer than |sizeof(Header)| bytes. | ||
- *data = reinterpret_cast<Header*>(message->mutable_payload()) + 1; | ||
- *num_data_bytes = message->payload_size() - sizeof(Header); | ||
+ // NOTE: Callers must guarantee that the payload in `message` must be at least | ||
+ // large enough to hold a Header. | ||
+ if (message.payload_size() < sizeof(Header)) | ||
+ return false; | ||
+ *data = reinterpret_cast<Header*>(message.mutable_payload()) + 1; | ||
+ *num_data_bytes = message.payload_size() - sizeof(Header); | ||
+ return true; | ||
} | ||
|
||
void NodeChannel::Start() { | ||
diff --git a/mojo/core/node_channel.h b/mojo/core/node_channel.h | ||
index 74306a56359cdc44b79797e6cc3c134b8029d69e..d2a273811c6db1ad8a37bd69fa81eb99d7ba4177 100644 | ||
--- a/mojo/core/node_channel.h | ||
+++ b/mojo/core/node_channel.h | ||
@@ -90,7 +90,9 @@ class MOJO_SYSTEM_IMPL_EXPORT NodeChannel | ||
void** payload, | ||
size_t num_handles); | ||
|
||
- static void GetEventMessageData(Channel::Message* message, | ||
+ // Retrieves address and size of an Event message's underlying message data. | ||
+ // Returns `false` if the message is not a valid Event message. | ||
+ static bool GetEventMessageData(Channel::Message& message, | ||
void** data, | ||
size_t* num_data_bytes); | ||
|
||
diff --git a/mojo/core/node_controller.cc b/mojo/core/node_controller.cc | ||
index 86b397df1431b2e83ccfceafe0af99e2f6d38c9e..351a34a40f0613ce167fd8158f5e3f494e95ce5e 100644 | ||
--- a/mojo/core/node_controller.cc | ||
+++ b/mojo/core/node_controller.cc | ||
@@ -76,7 +76,9 @@ ports::ScopedEvent DeserializeEventMessage( | ||
Channel::MessagePtr channel_message) { | ||
void* data; | ||
size_t size; | ||
- NodeChannel::GetEventMessageData(channel_message.get(), &data, &size); | ||
+ bool valid = NodeChannel::GetEventMessageData(*channel_message, &data, &size); | ||
+ if (!valid) | ||
+ return nullptr; | ||
auto event = ports::Event::Deserialize(data, size); | ||
if (!event) | ||
return nullptr; | ||
diff --git a/mojo/core/user_message_impl.cc b/mojo/core/user_message_impl.cc | ||
index 1f3f30abb4bbaca15513730691a0d52dfaebe0d7..f96c92077ee19dab8676e120684c6ac2f224358f 100644 | ||
--- a/mojo/core/user_message_impl.cc | ||
+++ b/mojo/core/user_message_impl.cc | ||
@@ -417,7 +417,14 @@ Channel::MessagePtr UserMessageImpl::FinalizeEventMessage( | ||
if (channel_message) { | ||
void* data; | ||
size_t size; | ||
- NodeChannel::GetEventMessageData(channel_message.get(), &data, &size); | ||
+ // The `channel_message` must either be produced locally or must have | ||
+ // already been validated by the caller, as is done for example by | ||
+ // NodeController::DeserializeEventMessage before | ||
+ // NodeController::OnBroadcast re-serializes each copy of the message it | ||
+ // received. | ||
+ bool result = | ||
+ NodeChannel::GetEventMessageData(*channel_message, &data, &size); | ||
+ DCHECK(result); | ||
message_event->Serialize(data); | ||
} | ||
|