mirror of https://github.com/electron/electron
182 lines
7.7 KiB
Diff
182 lines
7.7 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Ken Rockot <rockot@google.com>
|
|
Date: Mon, 23 Sep 2024 19:26:24 +0000
|
|
Subject: ipcz: Validate link state fragment before adoption
|
|
|
|
Fixed: 368208152
|
|
Change-Id: I0e2ece4b0857b225d229134b2e55abc3e08348ee
|
|
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5876623
|
|
Commit-Queue: Ken Rockot <rockot@google.com>
|
|
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
|
|
Cr-Commit-Position: refs/heads/main@{#1358968}
|
|
|
|
diff --git a/third_party/ipcz/src/ipcz/node_link.cc b/third_party/ipcz/src/ipcz/node_link.cc
|
|
index 8c378b1ad4d6b06b4d4e80fc99483edea043dde9..ed8d13fe801baed07d50e7458f55e5c0c2e18ccb 100644
|
|
--- a/third_party/ipcz/src/ipcz/node_link.cc
|
|
+++ b/third_party/ipcz/src/ipcz/node_link.cc
|
|
@@ -37,21 +37,6 @@
|
|
|
|
namespace ipcz {
|
|
|
|
-namespace {
|
|
-
|
|
-template <typename T>
|
|
-FragmentRef<T> MaybeAdoptFragmentRef(NodeLinkMemory& memory,
|
|
- const FragmentDescriptor& descriptor) {
|
|
- if (descriptor.is_null() || descriptor.size() < sizeof(T) ||
|
|
- descriptor.offset() % 8 != 0) {
|
|
- return {};
|
|
- }
|
|
-
|
|
- return memory.AdoptFragmentRef<T>(memory.GetFragment(descriptor));
|
|
-}
|
|
-
|
|
-} // namespace
|
|
-
|
|
// static
|
|
Ref<NodeLink> NodeLink::CreateActive(Ref<Node> node,
|
|
LinkSide link_side,
|
|
@@ -703,8 +688,8 @@ bool NodeLink::OnAcceptBypassLink(msg::AcceptBypassLink& accept) {
|
|
return true;
|
|
}
|
|
|
|
- auto link_state = MaybeAdoptFragmentRef<RouterLinkState>(
|
|
- memory(), accept.params().new_link_state_fragment);
|
|
+ auto link_state = memory().AdoptFragmentRefIfValid<RouterLinkState>(
|
|
+ accept.params().new_link_state_fragment);
|
|
if (link_state.is_null()) {
|
|
// Bypass links must always come with a valid fragment for their
|
|
// RouterLinkState. If one has not been provided, that's a validation
|
|
@@ -746,8 +731,8 @@ bool NodeLink::OnBypassPeerWithLink(msg::BypassPeerWithLink& bypass) {
|
|
return true;
|
|
}
|
|
|
|
- auto link_state = MaybeAdoptFragmentRef<RouterLinkState>(
|
|
- memory(), bypass.params().new_link_state_fragment);
|
|
+ auto link_state = memory().AdoptFragmentRefIfValid<RouterLinkState>(
|
|
+ bypass.params().new_link_state_fragment);
|
|
if (link_state.is_null()) {
|
|
return false;
|
|
}
|
|
diff --git a/third_party/ipcz/src/ipcz/node_link_memory.h b/third_party/ipcz/src/ipcz/node_link_memory.h
|
|
index df8010b595fdecea8959d2277da22ec734728e43..ba04a7c03da0d7f0bb5424b858a1bea7801ab9b4 100644
|
|
--- a/third_party/ipcz/src/ipcz/node_link_memory.h
|
|
+++ b/third_party/ipcz/src/ipcz/node_link_memory.h
|
|
@@ -86,14 +86,29 @@ class NodeLinkMemory : public RefCounted<NodeLinkMemory> {
|
|
// with the same BufferId and dimensions as `descriptor`.
|
|
Fragment GetFragment(const FragmentDescriptor& descriptor);
|
|
|
|
- // Adopts an existing reference to a RefCountedFragment within `fragment`.
|
|
- // This does NOT increment the ref count of the RefCountedFragment.
|
|
+ // Adopts an existing reference to a RefCountedFragment within `fragment`,
|
|
+ // which must be a valid, properly aligned, and sufficiently sized fragment to
|
|
+ // hold a T. This does NOT increment the ref count of the RefCountedFragment.
|
|
template <typename T>
|
|
FragmentRef<T> AdoptFragmentRef(const Fragment& fragment) {
|
|
ABSL_ASSERT(sizeof(T) <= fragment.size());
|
|
return FragmentRef<T>(kAdoptExistingRef, WrapRefCounted(this), fragment);
|
|
}
|
|
|
|
+ // Attempts to adopt an existing reference to a RefCountedFragment located at
|
|
+ // `fragment`. Returns null if the fragment descriptor is null, misaligned,
|
|
+ // or of insufficient size. This does NOT increment the ref count of the
|
|
+ // RefCountedFragment.
|
|
+ template <typename T>
|
|
+ FragmentRef<T> AdoptFragmentRefIfValid(const FragmentDescriptor& descriptor) {
|
|
+ if (descriptor.is_null() || descriptor.size() < sizeof(T) ||
|
|
+ descriptor.offset() % 8 != 0) {
|
|
+ return {};
|
|
+ }
|
|
+
|
|
+ return AdoptFragmentRef<T>(GetFragment(descriptor));
|
|
+ }
|
|
+
|
|
// Adds a new buffer to the underlying BufferPool to use as additional
|
|
// allocation capacity for blocks of size `block_size`. Note that the
|
|
// contents of the mapped region must already be initialized as a
|
|
diff --git a/third_party/ipcz/src/ipcz/node_link_memory_test.cc b/third_party/ipcz/src/ipcz/node_link_memory_test.cc
|
|
index 8a515c1fa5bf2b49067f8538a3e7d08a0d859a4b..2f8cdd38d4f9a3cfbcb9570104748e9887647463 100644
|
|
--- a/third_party/ipcz/src/ipcz/node_link_memory_test.cc
|
|
+++ b/third_party/ipcz/src/ipcz/node_link_memory_test.cc
|
|
@@ -303,5 +303,54 @@ TEST_F(NodeLinkMemoryTest, ParcelDataAllocation) {
|
|
node_c->Close();
|
|
}
|
|
|
|
+struct TestObject : public RefCountedFragment {
|
|
+ public:
|
|
+ int x;
|
|
+ int y;
|
|
+};
|
|
+
|
|
+TEST_F(NodeLinkMemoryTest, AdoptFragmentRefIfValid) {
|
|
+ auto object = memory_a().AdoptFragmentRef<TestObject>(
|
|
+ memory_a().AllocateFragment(sizeof(TestObject)));
|
|
+ object->x = 5;
|
|
+ object->y = 42;
|
|
+
|
|
+ const FragmentDescriptor valid_descriptor(object.fragment().buffer_id(),
|
|
+ object.fragment().offset(),
|
|
+ sizeof(TestObject));
|
|
+
|
|
+ const FragmentDescriptor null_descriptor(
|
|
+ kInvalidBufferId, valid_descriptor.offset(), valid_descriptor.size());
|
|
+ EXPECT_TRUE(memory_a()
|
|
+ .AdoptFragmentRefIfValid<TestObject>(null_descriptor)
|
|
+ .is_null());
|
|
+
|
|
+ const FragmentDescriptor empty_descriptor(
|
|
+ valid_descriptor.buffer_id(), valid_descriptor.offset(), /*size=*/0);
|
|
+ EXPECT_TRUE(memory_a()
|
|
+ .AdoptFragmentRefIfValid<TestObject>(empty_descriptor)
|
|
+ .is_null());
|
|
+
|
|
+ const FragmentDescriptor short_descriptor(valid_descriptor.buffer_id(),
|
|
+ valid_descriptor.offset(),
|
|
+ sizeof(TestObject) - 4);
|
|
+ EXPECT_TRUE(memory_a()
|
|
+ .AdoptFragmentRefIfValid<TestObject>(short_descriptor)
|
|
+ .is_null());
|
|
+
|
|
+ const FragmentDescriptor unaligned_descriptor(valid_descriptor.buffer_id(),
|
|
+ valid_descriptor.offset() + 2,
|
|
+ valid_descriptor.size() - 2);
|
|
+ EXPECT_TRUE(memory_a()
|
|
+ .AdoptFragmentRefIfValid<TestObject>(unaligned_descriptor)
|
|
+ .is_null());
|
|
+
|
|
+ const auto adopted_object =
|
|
+ memory_a().AdoptFragmentRefIfValid<TestObject>(valid_descriptor);
|
|
+ ASSERT_TRUE(adopted_object.is_addressable());
|
|
+ EXPECT_EQ(5, adopted_object->x);
|
|
+ EXPECT_EQ(42, adopted_object->y);
|
|
+}
|
|
+
|
|
} // namespace
|
|
} // namespace ipcz
|
|
diff --git a/third_party/ipcz/src/ipcz/router.cc b/third_party/ipcz/src/ipcz/router.cc
|
|
index 79c443d942c6613ea8a52990b93c1811e2d3d166..b1dc593427ecae67b6758edd82257f88daefcde1 100644
|
|
--- a/third_party/ipcz/src/ipcz/router.cc
|
|
+++ b/third_party/ipcz/src/ipcz/router.cc
|
|
@@ -765,12 +765,16 @@ Ref<Router> Router::Deserialize(const RouterDescriptor& descriptor,
|
|
? descriptor.decaying_incoming_sequence_length
|
|
: descriptor.next_incoming_sequence_number);
|
|
|
|
+ auto link_state =
|
|
+ from_node_link.memory().AdoptFragmentRefIfValid<RouterLinkState>(
|
|
+ descriptor.new_link_state_fragment);
|
|
+ if (link_state.is_null()) {
|
|
+ // Central links require a valid link state fragment.
|
|
+ return nullptr;
|
|
+ }
|
|
new_outward_link = from_node_link.AddRemoteRouterLink(
|
|
- context, descriptor.new_sublink,
|
|
- from_node_link.memory().AdoptFragmentRef<RouterLinkState>(
|
|
- from_node_link.memory().GetFragment(
|
|
- descriptor.new_link_state_fragment)),
|
|
- LinkType::kCentral, LinkSide::kB, router);
|
|
+ context, descriptor.new_sublink, std::move(link_state), LinkType::kCentral,
|
|
+ LinkSide::kB, router);
|
|
if (!new_outward_link) {
|
|
return nullptr;
|
|
}
|