From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Ken Rockot <rockot@google.com>
Date: Mon, 30 Sep 2024 06:33:13 +0000
Subject: ipcz: Validate link state fragment before adoption

(cherry picked from commit c333ed99544992f66e6e03621fa938d75ad01f70)

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-Original-Commit-Position: refs/heads/main@{#1358968}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5893005
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Ken Rockot <rockot@google.com>
Cr-Commit-Position: refs/branch-heads/6613@{#2136}
Cr-Branched-From: 03c1799e6f9c7239802827eab5e935b9e14fceae-refs/heads/main@{#1331488}

diff --git a/third_party/ipcz/src/ipcz/node_link.cc b/third_party/ipcz/src/ipcz/node_link.cc
index 420377ac3d0c5a1f745e8c5d0030c21b5d3639f2..412cedc59eaabd79a35d7e299e42f720de9ac148 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,
@@ -721,8 +706,8 @@ bool NodeLink::OnAcceptBypassLink(msg::AcceptBypassLink& accept) {
     return true;
   }
 
-  auto link_state = MaybeAdoptFragmentRef<RouterLinkState>(
-      memory(), accept.v0()->new_link_state_fragment);
+  auto link_state = memory().AdoptFragmentRefIfValid<RouterLinkState>(
+      accept.v0()->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
@@ -764,8 +749,8 @@ bool NodeLink::OnBypassPeerWithLink(msg::BypassPeerWithLink& bypass) {
     return true;
   }
 
-  auto link_state = MaybeAdoptFragmentRef<RouterLinkState>(
-      memory(), bypass.v0()->new_link_state_fragment);
+  auto link_state = memory().AdoptFragmentRefIfValid<RouterLinkState>(
+      bypass.v0()->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 c5e3d6494580412abf71e75f0736ad5a82abe95c..f457757cb703d56b8dd7d152b10f3fe6f8f8c84e 100644
--- a/third_party/ipcz/src/ipcz/node_link_memory.h
+++ b/third_party/ipcz/src/ipcz/node_link_memory.h
@@ -98,14 +98,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 bcdd45ee866ec39d557ee1c762af04ae0af26b6a..fd51b78a2a4475b54f047310da4c91e40a61342e 100644
--- a/third_party/ipcz/src/ipcz/node_link_memory_test.cc
+++ b/third_party/ipcz/src/ipcz/node_link_memory_test.cc
@@ -306,5 +306,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;
       }
