summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKen Rockot <rockot@google.com>2021-03-31 18:44:06 +0000
committerMichael Brüning <michael.bruning@qt.io>2021-04-09 10:50:51 +0000
commita775361f525f4a2054eec3c75be57bb0aa966356 (patch)
treeed4ec573554cb8013ab48085b270248358e24d8b
parent5cc54b6c60ee4cdb3ca49076d8d2baf53f437596 (diff)
downloadqtwebengine-chromium-a775361f525f4a2054eec3c75be57bb0aa966356.tar.gz
[Backport] CVE-2021-21198: Out of bounds read in IPC
Manual backport of patch originally reviewed on https://chromium-review.googlesource.com/c/chromium/src/+/2779918: Don't use BigBuffer for IPC::Message transport M86 merge conflicts and resolution: * ipc/ipc_message_pipe_reader.cc Fixed extra include. (cherry picked from commit 85bd7c88523545ab0e497d5e7b3e929793813358) (cherry picked from commit fad3b9ffe7c7ff82909d911c573bd185aa3b3b50) Fixed: 1184399 Change-Id: Iddd91ae8d7ae63022b61c96239f5e39261dfb735 Commit-Queue: Ken Rockot <rockot@google.com> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Original-Original-Commit-Position: refs/heads/master@{#860010} Auto-Submit: Ken Rockot <rockot@google.com> Reviewed-by: Adrian Taylor <adetaylor@chromium.org> Reviewed-by: Alex Gough <ajgo@chromium.org> Commit-Queue: Alex Gough <ajgo@chromium.org> Cr-Original-Commit-Position: refs/branch-heads/4389@{#1597} Cr-Original-Branched-From: 9251c5db2b6d5a59fe4eac7aafa5fed37c139bb7-refs/heads/master@{#843830} Reviewed-by: Victor-Gabriel Savu <vsavu@google.com> Reviewed-by: Artem Sumaneev <asumaneev@google.com> Reviewed-by: Ken Rockot <rockot@google.com> Auto-Submit: Artem Sumaneev <asumaneev@google.com> Commit-Queue: Artem Sumaneev <asumaneev@google.com> Cr-Commit-Position: refs/branch-heads/4240@{#1587} Cr-Branched-From: f297677702651916bbf65e59c0d4bbd4ce57d1ee-refs/heads/master@{#800218} Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
-rw-r--r--chromium/ipc/BUILD.gn5
-rw-r--r--chromium/ipc/ipc.mojom4
-rw-r--r--chromium/ipc/ipc_message_pipe_reader.cc10
-rw-r--r--chromium/ipc/message.typemap5
-rw-r--r--chromium/ipc/message_mojom_traits.cc16
-rw-r--r--chromium/ipc/message_mojom_traits.h4
-rw-r--r--chromium/ipc/message_view.cc12
-rw-r--r--chromium/ipc/message_view.h19
8 files changed, 25 insertions, 50 deletions
diff --git a/chromium/ipc/BUILD.gn b/chromium/ipc/BUILD.gn
index bfebba750fb..4fc97bac279 100644
--- a/chromium/ipc/BUILD.gn
+++ b/chromium/ipc/BUILD.gn
@@ -181,10 +181,7 @@ mojom_component("mojom") {
sources = [
"ipc.mojom",
]
- public_deps = [
- "//mojo/public/interfaces/bindings",
- "//mojo/public/mojom/base",
- ]
+ public_deps = [ "//mojo/public/interfaces/bindings" ]
# Don't generate a variant sources since we depend on generated internal
# bindings types and we don't generate or build variants of those.
diff --git a/chromium/ipc/ipc.mojom b/chromium/ipc/ipc.mojom
index 9631ce8fd23..4fb8881fcfc 100644
--- a/chromium/ipc/ipc.mojom
+++ b/chromium/ipc/ipc.mojom
@@ -4,7 +4,6 @@
module IPC.mojom;
-import "mojo/public/mojom/base/big_buffer.mojom";
import "mojo/public/interfaces/bindings/native_struct.mojom";
// A placeholder interface type since we don't yet support generic associated
@@ -14,7 +13,7 @@ interface GenericInterface {};
// Typemapped such that arbitrarily large IPC::Message objects can be sent and
// received with minimal copying.
struct Message {
- mojo_base.mojom.BigBuffer buffer;
+ array<uint8> bytes;
array<mojo.native.SerializedHandle>? handles;
};
@@ -24,6 +23,7 @@ interface Channel {
SetPeerPid(int32 pid);
// Transmits a classical Chrome IPC message.
+ [UnlimitedSize]
Receive(Message message);
// Requests a Channel-associated interface.
diff --git a/chromium/ipc/ipc_message_pipe_reader.cc b/chromium/ipc/ipc_message_pipe_reader.cc
index b055d413f45..72d20244547 100644
--- a/chromium/ipc/ipc_message_pipe_reader.cc
+++ b/chromium/ipc/ipc_message_pipe_reader.cc
@@ -10,6 +10,7 @@
#include "base/bind.h"
#include "base/bind_helpers.h"
+#include "base/containers/span.h"
#include "base/location.h"
#include "base/logging.h"
#include "base/macros.h"
@@ -63,7 +64,9 @@ bool MessagePipeReader::Send(std::unique_ptr<Message> message) {
if (!sender_)
return false;
- sender_->Receive(MessageView(*message, std::move(handles)));
+ base::span<const uint8_t> bytes(static_cast<const uint8_t*>(message->data()),
+ message->size());
+ sender_->Receive(MessageView(bytes, std::move(handles)));
DVLOG(4) << "Send " << message->type() << ": " << message->size();
return true;
}
@@ -82,11 +85,12 @@ void MessagePipeReader::SetPeerPid(int32_t peer_pid) {
}
void MessagePipeReader::Receive(MessageView message_view) {
- if (!message_view.size()) {
+ if (message_view.bytes().empty()) {
delegate_->OnBrokenDataReceived();
return;
}
- Message message(message_view.data(), message_view.size());
+ Message message(reinterpret_cast<const char*>(message_view.bytes().data()),
+ message_view.bytes().size());
if (!message.IsValid()) {
delegate_->OnBrokenDataReceived();
return;
diff --git a/chromium/ipc/message.typemap b/chromium/ipc/message.typemap
index e150786b3ff..a99406ffece 100644
--- a/chromium/ipc/message.typemap
+++ b/chromium/ipc/message.typemap
@@ -11,8 +11,5 @@ sources = [
"//ipc/message_view.cc",
"//ipc/message_view.h",
]
-public_deps = [
- "//ipc:message_support",
- "//mojo/public/cpp/base:shared_typemap_traits",
-]
+public_deps = [ "//ipc:message_support" ]
type_mappings = [ "IPC.mojom.Message=IPC::MessageView[move_only]" ]
diff --git a/chromium/ipc/message_mojom_traits.cc b/chromium/ipc/message_mojom_traits.cc
index 4aab9248e9f..d8ad4a2f919 100644
--- a/chromium/ipc/message_mojom_traits.cc
+++ b/chromium/ipc/message_mojom_traits.cc
@@ -4,15 +4,13 @@
#include "ipc/message_mojom_traits.h"
-#include "mojo/public/cpp/base/big_buffer_mojom_traits.h"
-
namespace mojo {
// static
-mojo_base::BigBufferView
-StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::buffer(
+base::span<const uint8_t>
+StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::bytes(
IPC::MessageView& view) {
- return view.TakeBufferView();
+ return view.bytes();
}
// static
@@ -26,14 +24,14 @@ StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::handles(
bool StructTraits<IPC::mojom::MessageDataView, IPC::MessageView>::Read(
IPC::mojom::MessageDataView data,
IPC::MessageView* out) {
- mojo_base::BigBufferView buffer_view;
- if (!data.ReadBuffer(&buffer_view))
- return false;
+ mojo::ArrayDataView<uint8_t> bytes;
+ data.GetBytesDataView(&bytes);
+
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles;
if (!data.ReadHandles(&handles))
return false;
- *out = IPC::MessageView(std::move(buffer_view), std::move(handles));
+ *out = IPC::MessageView(bytes, std::move(handles));
return true;
}
diff --git a/chromium/ipc/message_mojom_traits.h b/chromium/ipc/message_mojom_traits.h
index 617ffbe3730..6b5064a1219 100644
--- a/chromium/ipc/message_mojom_traits.h
+++ b/chromium/ipc/message_mojom_traits.h
@@ -7,10 +7,10 @@
#include <vector>
+#include "base/containers/span.h"
#include "base/optional.h"
#include "ipc/ipc.mojom-shared.h"
#include "ipc/message_view.h"
-#include "mojo/public/cpp/base/big_buffer.h"
#include "mojo/public/cpp/bindings/struct_traits.h"
#include "mojo/public/interfaces/bindings/native_struct.mojom.h"
@@ -19,7 +19,7 @@ namespace mojo {
template <>
class StructTraits<IPC::mojom::MessageDataView, IPC::MessageView> {
public:
- static mojo_base::BigBufferView buffer(IPC::MessageView& view);
+ static base::span<const uint8_t> bytes(IPC::MessageView& view);
static base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles(
IPC::MessageView& view);
diff --git a/chromium/ipc/message_view.cc b/chromium/ipc/message_view.cc
index d6ff4fb05f7..a8b9e43dc92 100644
--- a/chromium/ipc/message_view.cc
+++ b/chromium/ipc/message_view.cc
@@ -9,17 +9,9 @@ namespace IPC {
MessageView::MessageView() = default;
MessageView::MessageView(
- const Message& message,
+ base::span<const uint8_t> bytes,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
- : buffer_view_(base::make_span<const uint8_t>(
- static_cast<const uint8_t*>(message.data()),
- message.size())),
- handles_(std::move(handles)) {}
-
-MessageView::MessageView(
- mojo_base::BigBufferView buffer_view,
- base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles)
- : buffer_view_(std::move(buffer_view)), handles_(std::move(handles)) {}
+ : bytes_(bytes), handles_(std::move(handles)) {}
MessageView::MessageView(MessageView&&) = default;
diff --git a/chromium/ipc/message_view.h b/chromium/ipc/message_view.h
index 23e5da6e16f..17efcdaa269 100644
--- a/chromium/ipc/message_view.h
+++ b/chromium/ipc/message_view.h
@@ -11,7 +11,6 @@
#include "base/containers/span.h"
#include "base/macros.h"
#include "ipc/ipc_message.h"
-#include "mojo/public/cpp/base/big_buffer.h"
#include "mojo/public/interfaces/bindings/native_struct.mojom.h"
namespace IPC {
@@ -20,32 +19,20 @@ class COMPONENT_EXPORT(IPC_MOJOM) MessageView {
public:
MessageView();
MessageView(
- const Message& message,
- base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
- MessageView(
- mojo_base::BigBufferView buffer_view,
+ base::span<const uint8_t> bytes,
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles);
MessageView(MessageView&&);
~MessageView();
MessageView& operator=(MessageView&&);
- const char* data() const {
- return reinterpret_cast<const char*>(buffer_view_.data().data());
- }
-
- uint32_t size() const {
- return static_cast<uint32_t>(buffer_view_.data().size());
- }
-
- mojo_base::BigBufferView TakeBufferView() { return std::move(buffer_view_); }
-
+ base::span<const uint8_t> bytes() const { return bytes_; }
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> TakeHandles() {
return std::move(handles_);
}
private:
- mojo_base::BigBufferView buffer_view_;
+ base::span<const uint8_t> bytes_;
base::Optional<std::vector<mojo::native::SerializedHandlePtr>> handles_;
DISALLOW_COPY_AND_ASSIGN(MessageView);