diff options
author | Ken Rockot <rockot@google.com> | 2021-04-20 15:46:33 +0000 |
---|---|---|
committer | Michael BrĂ¼ning <michael.bruning@qt.io> | 2021-04-21 10:48:22 +0000 |
commit | f6b6811b9e04052a95264b8612d19659aab19fca (patch) | |
tree | 54bb95ed042fc112dbccac6d2879f26d753fea26 | |
parent | 116e1489a7089f01d5eb220b60c741874c2b3cbc (diff) |
[Backport] CVE-2021-21223: Integer overflow in Mojo
Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/2837712:
M86-LTS: 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-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Ken Rockot <rockot@google.com>
Cr-Original-Commit-Position: refs/heads/master@{#870238}
Owners-Override: Achuith Bhandarkar <achuith@chromium.org>
Auto-Submit: Achuith Bhandarkar <achuith@chromium.org>
Reviewed-by: Artem Sumaneev <asumaneev@google.com>
Commit-Queue: Achuith Bhandarkar <achuith@chromium.org>
Cr-Commit-Position: refs/branch-heads/4240@{#1614}
Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218}
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
-rw-r--r-- | chromium/mojo/core/node_channel.cc | 13 | ||||
-rw-r--r-- | chromium/mojo/core/node_channel.h | 4 | ||||
-rw-r--r-- | chromium/mojo/core/node_controller.cc | 4 | ||||
-rw-r--r-- | chromium/mojo/core/user_message_impl.cc | 9 |
4 files changed, 22 insertions, 8 deletions
diff --git a/chromium/mojo/core/node_channel.cc b/chromium/mojo/core/node_channel.cc index 5db339ec802..901c0f76c63 100644 --- a/chromium/mojo/core/node_channel.cc +++ b/chromium/mojo/core/node_channel.cc @@ -191,13 +191,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/chromium/mojo/core/node_channel.h b/chromium/mojo/core/node_channel.h index 58ab42bd01f..7ae08e3e731 100644 --- a/chromium/mojo/core/node_channel.h +++ b/chromium/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/chromium/mojo/core/node_controller.cc b/chromium/mojo/core/node_controller.cc index a297c500215..38ebaea2d98 100644 --- a/chromium/mojo/core/node_controller.cc +++ b/chromium/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/chromium/mojo/core/user_message_impl.cc b/chromium/mojo/core/user_message_impl.cc index bd3a6766e9d..e40682b0718 100644 --- a/chromium/mojo/core/user_message_impl.cc +++ b/chromium/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); } |