[Backport] CVE-2023-1810: Heap buffer overflow in Visuals

Manual cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/4298330:
Add CHECKs in HostFrameSinkManager

It looks like it's possible for a compromised renderer to get multiple
things to register the same FrameSinkId with HostFrameSinkManager. This
violates assumptions around ownership so turn DCHECKs here into CHECKs.
Also convert DCHECKs into CHECKs for registering/unregistering frame
sink hierarchy just in case.

(cherry picked from commit a707ac2d95e4726f4cf0267c9b0c038926c2a691)

Bug: 1414018
Change-Id: If948e758a8484024666f4066360620bc3a9cb493
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4283141
Reviewed-by: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: Jonathan Ross <jonross@chromium.org>
Commit-Queue: Kyle Charbonneau <kylechar@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1109533}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4298330
Cr-Commit-Position: refs/branch-heads/5615@{#69}
Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/475987
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

2 files changed, 9 insertions, 9 deletions

diff --git a/chromium/components/viz/host/host_frame_sink_manager.cc b/chromium/components/viz/host/host_frame_sink_manager.cc
index 9e7eca85a78..d7085046e2e 100644
--- a/chromium/components/viz/host/host_frame_sink_manager.cc
+++ b/chromium/components/viz/host/host_frame_sink_manager.cc
@@ -61,7 +61,7 @@ void HostFrameSinkManager::RegisterFrameSinkId(
   DCHECK(client);
 
   FrameSinkData& data = frame_sink_data_map_[frame_sink_id];
-  DCHECK(!data.IsFrameSinkRegistered());
+  CHECK(!data.IsFrameSinkRegistered());
   DCHECK(!data.has_created_compositor_frame_sink);
   data.client = client;
   data.report_activation = report_activation;
@@ -80,7 +80,7 @@ void HostFrameSinkManager::InvalidateFrameSinkId(
   DCHECK(frame_sink_id.is_valid());
 
   FrameSinkData& data = frame_sink_data_map_[frame_sink_id];
-  DCHECK(data.IsFrameSinkRegistered());
+  CHECK(data.IsFrameSinkRegistered());
 
   const bool destroy_synchronously =
 #if defined(TOOLKIT_QT)
@@ -208,6 +208,10 @@ bool HostFrameSinkManager::RegisterFrameSinkHierarchy(
   CHECK(parent_frame_sink_id.is_valid()) << parent_frame_sink_id;
   CHECK(child_frame_sink_id.is_valid()) << child_frame_sink_id;
 
+  FrameSinkData& parent_data = iter->second;
+  CHECK(!base::Contains(parent_data.children, child_frame_sink_id));
+  parent_data.children.push_back(child_frame_sink_id);
+
   // Register and store the parent.
   frame_sink_manager_->RegisterFrameSinkHierarchy(parent_frame_sink_id,
                                                   child_frame_sink_id);
@@ -216,10 +220,6 @@ bool HostFrameSinkManager::RegisterFrameSinkHierarchy(
   DCHECK(!base::Contains(child_data.parents, parent_frame_sink_id));
   child_data.parents.push_back(parent_frame_sink_id);
 
-  FrameSinkData& parent_data = iter->second;
-  DCHECK(!base::Contains(parent_data.children, child_frame_sink_id));
-  parent_data.children.push_back(child_frame_sink_id);
-
   return true;
 }
 
@@ -232,8 +232,8 @@ void HostFrameSinkManager::UnregisterFrameSinkHierarchy(
   base::Erase(child_data.parents, parent_frame_sink_id);
 
   FrameSinkData& parent_data = frame_sink_data_map_[parent_frame_sink_id];
-  DCHECK(base::Contains(parent_data.children, child_frame_sink_id));
-  base::Erase(parent_data.children, child_frame_sink_id);
+  size_t num_erased = base::Erase(parent_data.children, child_frame_sink_id);
+  CHECK_EQ(num_erased, 1u);
 
   if (frame_sink_manager_)
     frame_sink_manager_->UnregisterFrameSinkHierarchy(parent_frame_sink_id,
diff --git a/chromium/components/viz/service/frame_sinks/frame_sink_manager_impl.cc b/chromium/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
index 60c8f3402b1..5315e3cc441 100644
--- a/chromium/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
+++ b/chromium/components/viz/service/frame_sinks/frame_sink_manager_impl.cc
@@ -238,7 +238,7 @@ void FrameSinkManagerImpl::UnregisterFrameSinkHierarchy(
   }
 
   auto iter = frame_sink_source_map_.find(parent_frame_sink_id);
-  DCHECK(iter != frame_sink_source_map_.end());
+  CHECK(iter != frame_sink_source_map_.end());
 
   // Remove |child_frame_sink_id| from parents list of children.
   auto& mapping = iter->second;