From 6ab025cb13a7d2e2c3ca1890f989b5707d39c3bd Mon Sep 17 00:00:00 2001
From: Mason Freed <masonf@chromium.org>
Date: Mon, 14 Sep 2026 12:54:46 -0700
Subject: [PATCH] Ensure PopupWidgetHost popups cannot appear above the line of death

A compromised renderer could request popup widget host bounds (such as
those used by `<select>` and date/time or color pickers) that place the
popup over browser UI, including the omnibox. While popups may extend
outside their containing frames or the browser frame, they must not
extend above the "line of death" of the top-level main frame. Prior to
this CL, all of these pickers were able to be positioned (even by
non-compromised renderers, e.g. see crbug.com/40092820) above the
line of death, including on top of the omnibox.

This CL enforces a browser-side limit on PopupWidgetHost bounds in both
WebContentsImpl::ShowCreatedWidget and RenderWidgetHostImpl::
SetPopupBounds. The top coordinate (y) of the popup is clamped so that
it is at or below the top edge of the outermost main frame's view.

This is *already* the behavior of Firefox, and this behavior is not
specified/standardized. This change may affect the rendered behavior
of some (non-exploit) sites with very large menus, but the effect
should be small in the normal case.

Fixed: 424995036
Bug: 40086125,40092820,40065947,424995513,417555081
Change-Id: I11c76157ed87c55afa197a8157b3fcb7635b67de
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8379462
Auto-Submit: Mason Freed <masonf@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1697455}
---
diff --git a/content/browser/renderer_host/render_widget_host_delegate.cc b/content/browser/renderer_host/render_widget_host_delegate.cc
index 975cf9ae..942d2a5c 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.cc
+++ b/content/browser/renderer_host/render_widget_host_delegate.cc
@@ -188,6 +188,11 @@
   return false;
 }
 
+gfx::Rect RenderWidgetHostDelegate::ConstrainPopupBounds(
+    const gfx::Rect& bounds) {
+  return bounds;
+}
+
 int RenderWidgetHostDelegate::GetVirtualKeyboardResizeHeight() {
   return 0;
 }
diff --git a/content/browser/renderer_host/render_widget_host_delegate.h b/content/browser/renderer_host/render_widget_host_delegate.h
index 99fc4bb..74a2c46 100644
--- a/content/browser/renderer_host/render_widget_host_delegate.h
+++ b/content/browser/renderer_host/render_widget_host_delegate.h
@@ -398,6 +398,11 @@
                                  const gfx::Rect& initial_anchor_rect_in_dips) {
   }
 
+  // Constrain popup bounds so that the top of the popup is at or below the
+  // line of death (the top of the top-level main frame). See
+  // crbug.com/424995036.
+  virtual gfx::Rect ConstrainPopupBounds(const gfx::Rect& bounds);
+
   // Returns the amount that this view has been resized by a showing virtual
   // keyboard or 0 if the virtual keyboard is hidden or in a mode that doesn't
   // resize the view.
diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 9a1260b..f5eb880 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -2777,7 +2777,9 @@
   // same time until it acked the changes. Otherwise, if they simultaneously
   // change bounds, browser's bounds can be clobbered.
   if (view_ && !waiting_for_screen_rects_ack_) {
-    view_->SetBounds(bounds);
+    gfx::Rect constrained_bounds =
+        delegate_ ? delegate_->ConstrainPopupBounds(bounds) : bounds;
+    view_->SetBounds(constrained_bounds);
   }
   std::move(callback).Run();
 }
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index c9f40e6..80170c73 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -6070,6 +6070,24 @@
                                   nullptr);
 }
 
+gfx::Rect WebContentsImpl::ConstrainPopupBounds(const gfx::Rect& bounds) {
+  if (!base::FeatureList::IsEnabled(features::kLimitPopupWidgetHostPosition)) {
+    return bounds;
+  }
+  // Constrain popup bounds so that the top of the popup is at or below the line
+  // of death (the top of the top-level main frame). See crbug.com/424995036.
+  RenderWidgetHostView* view = GetTopLevelRenderWidgetHostView();
+  if (!view) {
+    return bounds;
+  }
+  gfx::Rect constrained_bounds = bounds;
+  int line_of_death = view->GetViewBounds().y();
+  if (constrained_bounds.y() < line_of_death) {
+    constrained_bounds.set_y(line_of_death);
+  }
+  return constrained_bounds;
+}
+
 void WebContentsImpl::ShowCreatedWidget(ChildProcessId process_id,
                                         int widget_route_id,
                                         const gfx::Rect& initial_rect,
@@ -6134,6 +6152,8 @@
                   bottom_right.y() - origin.y());
   }
 
+  transformed_rect = ConstrainPopupBounds(transformed_rect);
+
   RenderWidgetHostImpl* render_widget_host_impl = widget_host_view->host();
 
   // A background tab cannot show a popup over the active tab.
diff --git a/content/browser/web_contents/web_contents_impl.h b/content/browser/web_contents/web_contents_impl.h
index 21097745..58d38d1 100644
--- a/content/browser/web_contents/web_contents_impl.h
+++ b/content/browser/web_contents/web_contents_impl.h
@@ -1019,6 +1019,7 @@
                          int widget_route_id,
                          const gfx::Rect& initial_rect,
                          const gfx::Rect& initial_anchor_rect) override;
+  gfx::Rect ConstrainPopupBounds(const gfx::Rect& bounds) override;
   void CreateMediaPlayerHostForRenderFrameHost(
       RenderFrameHostImpl* frame_host,
       mojo::PendingAssociatedReceiver<media::mojom::MediaPlayerHost> receiver)
diff --git a/content/common/features.cc b/content/common/features.cc
index 60268e3b..7ac35b5 100644
--- a/content/common/features.cc
+++ b/content/common/features.cc
@@ -408,6 +408,10 @@
 // invalid (non-allowlisted) headers in a navigation request.
 BASE_FEATURE(kKillOnInvalidNavigationHeaders, base::FEATURE_ENABLED_BY_DEFAULT);
 
+// Kill switch for crbug.com/424995036. When enabled, the top of a
+// PopupWidgetHost is constrained to be at or below the line of death.
+BASE_FEATURE(kLimitPopupWidgetHostPosition, base::FEATURE_ENABLED_BY_DEFAULT);
+
 // Enables Local Network Access checks for all types of web workers.
 //
 // The exact checks run are the same as for other document subresources, and
diff --git a/content/common/features.h b/content/common/features.h
index 9b0eac19..9af9404 100644
--- a/content/common/features.h
+++ b/content/common/features.h
@@ -149,6 +149,7 @@
 
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kKeepChildProcessAfterIPCReset);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kKillOnInvalidNavigationHeaders);
+CONTENT_EXPORT BASE_DECLARE_FEATURE(kLimitPopupWidgetHostPosition);
 
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kLocalNetworkAccessForWorkers);
 CONTENT_EXPORT BASE_DECLARE_FEATURE(kLocalNetworkAccessForWorkersWarningOnly);
diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index 59ad382..48fef653d 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -479,6 +479,18 @@
     return &text_input_manager_;
   }
 
+  gfx::Rect ConstrainPopupBounds(const gfx::Rect& bounds) override {
+    if (constrain_popup_bounds_callback_) {
+      return constrain_popup_bounds_callback_.Run(bounds);
+    }
+    return RenderWidgetHostDelegate::ConstrainPopupBounds(bounds);
+  }
+
+  void set_constrain_popup_bounds_callback(
+      base::RepeatingCallback<gfx::Rect(const gfx::Rect&)> callback) {
+    constrain_popup_bounds_callback_ = std::move(callback);
+  }
+
   MOCK_METHOD(bool,
               IsWaitingForPointerLockPrompt,
               (RenderWidgetHostImpl * host),
@@ -571,6 +583,9 @@
 
   TextInputManager text_input_manager_;
 
+  base::RepeatingCallback<gfx::Rect(const gfx::Rect&)>
+      constrain_popup_bounds_callback_;
+
   VisibleTimeRequestTrigger visible_time_request_trigger_;
 };
 
@@ -963,6 +978,31 @@
   EXPECT_EQ(new_popup_view_bounds, view_->GetViewBounds());
 }
 
+TEST_F(RenderWidgetHostTest, SetPopupBoundsConstrainedByDelegate) {
+  ClearScreenRects();
+  base::RunLoop().RunUntilIdle();
+
+  // Default delegate implementation does not constrain bounds.
+  gfx::Rect unconstrained_bounds(5, 5, 20, 20);
+  EXPECT_EQ(delegate_->ConstrainPopupBounds(unconstrained_bounds),
+            unconstrained_bounds);
+
+  // Set a custom constraint on the delegate.
+  delegate_->set_constrain_popup_bounds_callback(
+      base::BindRepeating([](const gfx::Rect& bounds) {
+        gfx::Rect constrained = bounds;
+        if (constrained.y() < 100) {
+          constrained.set_y(100);
+        }
+        return constrained;
+      }));
+
+  // When SetPopupBounds is called, bounds are constrained by the delegate.
+  static_cast<blink::mojom::PopupWidgetHost*>(host_.get())
+      ->SetPopupBounds(unconstrained_bounds, base::DoNothing());
+  EXPECT_EQ(gfx::Rect(5, 100, 20, 20), view_->GetViewBounds());
+}
+
 TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) {
   ClearVisualProperties();
 
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 1f97117..5f5cdbf0 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -37,6 +37,7 @@
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/common/content_navigation_policy.h"
+#include "content/common/features.h"
 #include "content/common/frame.mojom.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/content_browser_client.h"
@@ -95,6 +96,7 @@
 #include "third_party/blink/public/mojom/page/page_visibility_state.mojom.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/ui_base_features.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/native_theme/native_theme.h"
@@ -4303,4 +4305,40 @@
             contents());
 }
 
+TEST_F(WebContentsImplTest, ConstrainPopupBounds) {
+  TestRenderWidgetHostView* view = static_cast<TestRenderWidgetHostView*>(
+      contents()->GetRenderWidgetHostView());
+  const int kLineOfDeath = 150;
+  view->SetBounds(gfx::Rect(50, kLineOfDeath, 800, 600));
+
+  // A popup whose top is above the line of death is clamped to the line of
+  // death.
+  gfx::Rect above_line_of_death(100, 50, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            gfx::Rect(100, kLineOfDeath, 200, 100));
+
+  // A popup whose top is exactly at the line of death is unchanged.
+  gfx::Rect at_line_of_death(100, kLineOfDeath, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(at_line_of_death),
+            at_line_of_death);
+
+  // A popup whose top is below the line of death is unchanged.
+  gfx::Rect below_line_of_death(100, 200, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(below_line_of_death),
+            below_line_of_death);
+
+  // If the line of death moves, clamping respects the new line of death.
+  const int kNewLineOfDeath = 250;
+  view->SetBounds(gfx::Rect(50, kNewLineOfDeath, 800, 600));
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            gfx::Rect(100, kNewLineOfDeath, 200, 100));
+
+  // When the feature is disabled, popups above the line of death are not
+  // clamped.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndDisableFeature(features::kLimitPopupWidgetHostPosition);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            above_line_of_death);
+}
+
 }  // namespace content

diff --git a/content/browser/renderer_host/render_widget_host_unittest.cc b/content/browser/renderer_host/render_widget_host_unittest.cc
index 59ad382..48fef653d 100644
--- a/content/browser/renderer_host/render_widget_host_unittest.cc
+++ b/content/browser/renderer_host/render_widget_host_unittest.cc
@@ -479,6 +479,18 @@
     return &text_input_manager_;
   }
 
+  gfx::Rect ConstrainPopupBounds(const gfx::Rect& bounds) override {
+    if (constrain_popup_bounds_callback_) {
+      return constrain_popup_bounds_callback_.Run(bounds);
+    }
+    return RenderWidgetHostDelegate::ConstrainPopupBounds(bounds);
+  }
+
+  void set_constrain_popup_bounds_callback(
+      base::RepeatingCallback<gfx::Rect(const gfx::Rect&)> callback) {
+    constrain_popup_bounds_callback_ = std::move(callback);
+  }
+
   MOCK_METHOD(bool,
               IsWaitingForPointerLockPrompt,
               (RenderWidgetHostImpl * host),
@@ -571,6 +583,9 @@
 
   TextInputManager text_input_manager_;
 
+  base::RepeatingCallback<gfx::Rect(const gfx::Rect&)>
+      constrain_popup_bounds_callback_;
+
   VisibleTimeRequestTrigger visible_time_request_trigger_;
 };
 
@@ -963,6 +978,31 @@
   EXPECT_EQ(new_popup_view_bounds, view_->GetViewBounds());
 }
 
+TEST_F(RenderWidgetHostTest, SetPopupBoundsConstrainedByDelegate) {
+  ClearScreenRects();
+  base::RunLoop().RunUntilIdle();
+
+  // Default delegate implementation does not constrain bounds.
+  gfx::Rect unconstrained_bounds(5, 5, 20, 20);
+  EXPECT_EQ(delegate_->ConstrainPopupBounds(unconstrained_bounds),
+            unconstrained_bounds);
+
+  // Set a custom constraint on the delegate.
+  delegate_->set_constrain_popup_bounds_callback(
+      base::BindRepeating([](const gfx::Rect& bounds) {
+        gfx::Rect constrained = bounds;
+        if (constrained.y() < 100) {
+          constrained.set_y(100);
+        }
+        return constrained;
+      }));
+
+  // When SetPopupBounds is called, bounds are constrained by the delegate.
+  static_cast<blink::mojom::PopupWidgetHost*>(host_.get())
+      ->SetPopupBounds(unconstrained_bounds, base::DoNothing());
+  EXPECT_EQ(gfx::Rect(5, 100, 20, 20), view_->GetViewBounds());
+}
+
 TEST_F(RenderWidgetHostTest, SynchronizeVisualProperties) {
   ClearVisualProperties();
diff --git a/content/browser/web_contents/web_contents_impl_unittest.cc b/content/browser/web_contents/web_contents_impl_unittest.cc
index 1f97117..5f5cdbf0 100644
--- a/content/browser/web_contents/web_contents_impl_unittest.cc
+++ b/content/browser/web_contents/web_contents_impl_unittest.cc
@@ -37,6 +37,7 @@
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/storage_partition_impl.h"
 #include "content/common/content_navigation_policy.h"
+#include "content/common/features.h"
 #include "content/common/frame.mojom.h"
 #include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/content_browser_client.h"
@@ -95,6 +96,7 @@
 #include "third_party/blink/public/mojom/page/page_visibility_state.mojom.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/ui_base_features.h"
+#include "ui/gfx/geometry/rect.h"
 #include "ui/gfx/geometry/size.h"
 #include "ui/gfx/geometry/skia_conversions.h"
 #include "ui/native_theme/native_theme.h"
@@ -4303,4 +4305,40 @@
             contents());
 }
 
+TEST_F(WebContentsImplTest, ConstrainPopupBounds) {
+  TestRenderWidgetHostView* view = static_cast<TestRenderWidgetHostView*>(
+      contents()->GetRenderWidgetHostView());
+  const int kLineOfDeath = 150;
+  view->SetBounds(gfx::Rect(50, kLineOfDeath, 800, 600));
+
+  // A popup whose top is above the line of death is clamped to the line of
+  // death.
+  gfx::Rect above_line_of_death(100, 50, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            gfx::Rect(100, kLineOfDeath, 200, 100));
+
+  // A popup whose top is exactly at the line of death is unchanged.
+  gfx::Rect at_line_of_death(100, kLineOfDeath, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(at_line_of_death),
+            at_line_of_death);
+
+  // A popup whose top is below the line of death is unchanged.
+  gfx::Rect below_line_of_death(100, 200, 200, 100);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(below_line_of_death),
+            below_line_of_death);
+
+  // If the line of death moves, clamping respects the new line of death.
+  const int kNewLineOfDeath = 250;
+  view->SetBounds(gfx::Rect(50, kNewLineOfDeath, 800, 600));
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            gfx::Rect(100, kNewLineOfDeath, 200, 100));
+
+  // When the feature is disabled, popups above the line of death are not
+  // clamped.
+  base::test::ScopedFeatureList feature_list;
+  feature_list.InitAndDisableFeature(features::kLimitPopupWidgetHostPosition);
+  EXPECT_EQ(contents()->ConstrainPopupBounds(above_line_of_death),
+            above_line_of_death);
+}
+
 }  // namespace content