From 926b0949a8dafd79619ef2b13bdff02e57ac289d Mon Sep 17 00:00:00 2001
From: David Pennington <[email protected]>
Date: Thu, 25 Jun 2026 15:25:21 -0700
Subject: [PATCH] Fix BrowserTabStripController UAF on focused-group auto-close

Because the focus feature can close a group, attempting to use the
ptr after the closure can result in UAF. we can either requery the
group from the tabstripmodel or we can reget the ptr, i chose to
use a weakptr to requery the validity of the ptr so that we dont have
to do another lookup.
Also added a regression test for VerticalTabStripController UAF

Bug: 513713427
Change-Id: I4b0d0fa580681e063a2ee33d5511e781ea68a740
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7998801
Reviewed-by: Charles Meng <[email protected]>
Commit-Queue: David Pennington <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1652763}
---

diff --git a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
index a95ae65..c1b3e9b 100644
--- a/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc
@@ -462,6 +462,13 @@
     }
   }
 
+  // Under the kTabGroupsFocusingAutoClose feature, switching the active tab or
+  // adding a new tab can cause the group to be automatically closed and
+  // synchronously destroyed. We must check that the group still exists.
+  if (!model_->group_model()->ContainsTabGroup(group)) {
+    return;
+  }
+
   if (origin != ToggleTabGroupCollapsedStateOrigin::kMenuAction ||
       should_toggle_group) {
     tabstrip_->ToggleTabGroup(group, !is_currently_collapsed, origin);
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller.cc b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller.cc
index d753488..05ae574 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller.cc
@@ -6,6 +6,7 @@
 
 #include <variant>
 
+#include "base/memory/weak_ptr.h"
 #include "base/metrics/histogram_functions.h"
 #include "base/metrics/user_metrics.h"
 #include "base/metrics/user_metrics_action.h"
@@ -287,6 +288,11 @@
   bool is_currently_collapsed = group->visual_data()->is_collapsed();
   bool should_toggle_group = true;
 
+  // We use a WeakPtr because switching the active tab or adding
+  // a new tab during a collapse operation can trigger the automatic
+  // closure of the group, which synchronously destroys the TabGroup.
+  base::WeakPtr<const TabGroup> weak_group = group->AsWeakPtr();
+
   tabs::TabInterface* active_tab = model_->GetActiveTab();
   if (!is_currently_collapsed && active_tab && !drag_handler_->IsDragging()) {
     if (active_tab->GetGroup() == group->id()) {
@@ -296,6 +302,7 @@
       // be created.
       const std::optional<int> next_active =
           model_->GetNextExpandedActiveTab(group->id());
+
       if (next_active.has_value()) {
         model_->ActivateTabAt(
             next_active.value(),
@@ -321,6 +328,10 @@
     }
   }
 
+  if (!weak_group) {
+    return;
+  }
+
   if (origin != ToggleTabGroupCollapsedStateOrigin::kMenuAction ||
       should_toggle_group) {
     model_->ChangeTabGroupVisuals(
diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
index c436ae2..7d91292f 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
@@ -487,4 +487,37 @@
   EXPECT_FALSE(tab0->GetGroup().has_value());
 }
 
+class VerticalTabStripControllerFocusingAutoCloseBrowserTest
+    : public VerticalTabsBrowserTestMixin<InProcessBrowserTest> {
+ public:
+  const std::vector<base::test::FeatureRefAndParams> GetEnabledFeatures()
+      override {
+    return {
+        {features::kTabGroupsFocusing,
+         {{"tab_groups_focusing_auto_close", "true"}}},
+        {tabs::kVerticalTabs, {}},
+    };
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(VerticalTabStripControllerFocusingAutoCloseBrowserTest,
+                       CollapseFocusedGroupTriggersUseAfterFree) {
+  AppendTab();
+
+  TabStripModel* model = browser()->tab_strip_model();
+  ASSERT_EQ(2, model->count());
+
+  tab_groups::TabGroupId group_id = model->AddToNewGroup({0});
+  TabGroup* group = model->group_model()->GetTabGroup(group_id);
+
+  model->SetFocusedGroup(group_id);
+  ASSERT_EQ(0, model->active_index());
+  ASSERT_EQ(group_id, model->GetFocusedGroup());
+
+  vertical_tab_strip_controller()->ToggleTabGroupCollapsedState(
+      group, ToggleTabGroupCollapsedStateOrigin::kMouse);
+
+  EXPECT_FALSE(model->group_model()->ContainsTabGroup(group_id));
+}
+
 }  // namespace
diff --git a/components/tabs/public/tab_group.h b/components/tabs/public/tab_group.h
index c6c9d323..1b55e2ce 100644
--- a/components/tabs/public/tab_group.h
+++ b/components/tabs/public/tab_group.h
@@ -11,6 +11,7 @@
 #include "base/callback_list.h"
 #include "base/functional/callback_forward.h"
 #include "base/memory/raw_ptr.h"
+#include "base/memory/weak_ptr.h"
 #include "components/tab_groups/tab_group_id.h"
 #include "components/tab_groups/tab_group_visual_data.h"
 #include "components/tabs/public/tab_collection.h"
@@ -50,6 +51,11 @@
   };
   virtual ~TabGroup();
 
+  base::WeakPtr<TabGroup> AsWeakPtr() { return weak_ptr_factory_.GetWeakPtr(); }
+  base::WeakPtr<const TabGroup> AsWeakPtr() const {
+    return weak_ptr_factory_.GetWeakPtr();
+  }
+
   const tab_groups::TabGroupId& id() const { return id_; }
   const tab_groups::TabGroupVisualData* visual_data() const {
     return visual_data_.get();
@@ -137,6 +143,8 @@
 
   bool is_closing_ = false;
   bool is_customized_ = false;
+
+  mutable base::WeakPtrFactory<TabGroup> weak_ptr_factory_{this};
 };
 
 #endif  // COMPONENTS_TABS_PUBLIC_TAB_GROUP_H_

diff --git a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
index c436ae2..7d91292f 100644
--- a/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
+++ b/chrome/browser/ui/views/tabs/vertical/vertical_tab_strip_controller_browsertest.cc
@@ -487,4 +487,37 @@
   EXPECT_FALSE(tab0->GetGroup().has_value());
 }
 
+class VerticalTabStripControllerFocusingAutoCloseBrowserTest
+    : public VerticalTabsBrowserTestMixin<InProcessBrowserTest> {
+ public:
+  const std::vector<base::test::FeatureRefAndParams> GetEnabledFeatures()
+      override {
+    return {
+        {features::kTabGroupsFocusing,
+         {{"tab_groups_focusing_auto_close", "true"}}},
+        {tabs::kVerticalTabs, {}},
+    };
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(VerticalTabStripControllerFocusingAutoCloseBrowserTest,
+                       CollapseFocusedGroupTriggersUseAfterFree) {
+  AppendTab();
+
+  TabStripModel* model = browser()->tab_strip_model();
+  ASSERT_EQ(2, model->count());
+
+  tab_groups::TabGroupId group_id = model->AddToNewGroup({0});
+  TabGroup* group = model->group_model()->GetTabGroup(group_id);
+
+  model->SetFocusedGroup(group_id);
+  ASSERT_EQ(0, model->active_index());
+  ASSERT_EQ(group_id, model->GetFocusedGroup());
+
+  vertical_tab_strip_controller()->ToggleTabGroupCollapsedState(
+      group, ToggleTabGroupCollapsedStateOrigin::kMouse);
+
+  EXPECT_FALSE(model->group_model()->ContainsTabGroup(group_id));
+}
+
 }  // namespace