From 7e257667273927316e93b112d1ec27830fdcdffe Mon Sep 17 00:00:00 2001
From: Tianyi Hu <[email protected]>
Date: Wed, 11 Mar 2026 18:16:21 -0700
Subject: [PATCH] Add browser-side validation for allow-modals sandbox attribute

RunJavaScriptDialog() does not check IsSandboxed(kModals) before
showing alert/confirm/prompt dialogs. Add a browser-side check
matching the allow-popups validation in CreateNewWindow(), so that
a sandboxed frame without allow-modals cannot show modal dialogs
even if the renderer is compromised.

Bug: 491676472
Change-Id: I8c3d3c0d2f167754399a67d6970313c3a6acbe64
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7656345
Reviewed-by: Alex Moshchuk <[email protected]>
Reviewed-by: Mark Pearson <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Alex Moshchuk <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1598123}
---

diff --git a/content/browser/bad_message.h b/content/browser/bad_message.h
index 372417d..54e2b1c 100644
--- a/content/browser/bad_message.h
+++ b/content/browser/bad_message.h
@@ -361,6 +361,7 @@
   RFH_NEW_ISOLATED_WEB_APP_PERMISSION_POLICIES = 333,
   RFH_CREATE_NEW_WINDOW_INVALID_DISPOSITION = 334,
   RFH_CREATE_NEW_WINDOW_FROM_SANDBOXED_FRAME = 335,
+  RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME = 336,
 
   // Please add new elements here. The naming convention is abbreviated class
   // name (e.g. RenderFrameHost becomes RFH) plus a unique description of the
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index c5ee522e..12304d4 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -7244,6 +7244,16 @@
     JavaScriptDialogType dialog_type,
     bool disable_third_party_subframe_suppresion,
     JavaScriptDialogCallback ipc_response_callback) {
+  // Sandboxed frames should only be allowed to show modal dialogs when they
+  // have the "allow-modals" attribute. This should have already been checked
+  // by the renderer process (see LocalDOMWindow::alert/confirm/prompt), and
+  // this browser-side check defends against compromised renderers.
+  if (IsSandboxed(network::mojom::WebSandboxFlags::kModals)) {
+    bad_message::ReceivedBadMessage(
+        GetProcess(), bad_message::RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME);
+    return;
+  }
+
   // Don't show the dialog if it's triggered on a non-active RenderFrameHost
   // or is contained in a Fenced Frame.
   if (!IsActive() || IsNestedWithinFencedFrame()) {
diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h
index 5664e61..7a1bdc10 100644
--- a/content/browser/renderer_host/render_frame_host_impl.h
+++ b/content/browser/renderer_host/render_frame_host_impl.h
@@ -3493,6 +3493,8 @@
                            CreateNewWindowWithInaccessibleFile);
   FRIEND_TEST_ALL_PREFIXES(SecurityExploitBrowserTest,
                            WindowOpenDisallowedFromSandboxedFrame);
+  FRIEND_TEST_ALL_PREFIXES(SecurityExploitBrowserTest,
+                           ModalDialogDisallowedFromSandboxedFrame);
   FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
                            RenderViewHostIsNotReusedAfterDelayedUnloadACK);
   FRIEND_TEST_ALL_PREFIXES(SitePerProcessBrowserTest,
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index ccf0287..d2fea7e 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1160,6 +1160,51 @@
   EXPECT_EQ(1u, Shell::windows().size());
 }
 
+// Regression test for browser-side validation of the allow-modals sandbox
+// attribute. A sandboxed frame without allow-modals should not be able to show
+// modal dialogs (alert, confirm, prompt). This is a variant of
+// WindowOpenDisallowedFromSandboxedFrame for the kModals sandbox flag.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       ModalDialogDisallowedFromSandboxedFrame) {
+  IsolateOrigin("b.com");
+
+  GURL main_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+  RenderFrameHostImpl* main_frame = root->current_frame_host();
+
+  // Create cross-site sandboxed child frame. The frame lacks the allow-modals
+  // attribute, so it should not be allowed to show modal dialogs.
+  GURL child_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  {
+    std::string js_str = base::StringPrintf(
+        "var frame = document.createElement('iframe'); "
+        "frame.sandbox = 'allow-scripts'; "
+        "frame.src = '%s'; "
+        "document.body.appendChild(frame);",
+        child_url.spec().c_str());
+    EXPECT_TRUE(ExecJs(main_frame, js_str));
+    ASSERT_TRUE(WaitForLoadStop(web_contents));
+  }
+
+  RenderFrameHostImpl* subframe = root->child_at(0)->current_frame_host();
+  EXPECT_TRUE(
+      subframe->GetSiteInstance()->GetSecurityPrincipal().IsSandboxed());
+  EXPECT_TRUE(subframe->IsSandboxed(network::mojom::WebSandboxFlags::kModals));
+
+  // Simulate that the b.com renderer is compromised and sends an IPC to show
+  // a modal dialog, bypassing renderer-side checks in LocalDOMWindow::alert().
+  // The browser process should detect this and terminate the renderer.
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(subframe->GetProcess());
+  subframe->RunModalAlertDialog(u"test", false, base::DoNothing());
+  EXPECT_EQ(bad_message::RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME,
+            kill_waiter.Wait());
+  EXPECT_FALSE(subframe->IsRenderFrameLive());
+}
+
 // Regression test for browser-side validation of POST submissions in
 // CreateNewWindow. See https://crbug.com/487768779.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
diff --git a/tools/metrics/histograms/metadata/stability/enums.xml b/tools/metrics/histograms/metadata/stability/enums.xml
index de97da0..3ea66989 100644
--- a/tools/metrics/histograms/metadata/stability/enums.xml
+++ b/tools/metrics/histograms/metadata/stability/enums.xml
@@ -501,6 +501,7 @@
   <int value="333" label="RFH_NEW_ISOLATED_WEB_APP_PERMISSION_POLICIES"/>
   <int value="334" label="RFH_CREATE_NEW_WINDOW_INVALID_DISPOSITION"/>
   <int value="335" label="RFH_CREATE_NEW_WINDOW_FROM_SANDBOXED_FRAME"/>
+  <int value="336" label="RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME"/>
 </enum>
 
 <enum name="BadMessageReasonExtensions">

diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index ccf0287..d2fea7e 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -1160,6 +1160,51 @@
   EXPECT_EQ(1u, Shell::windows().size());
 }
 
+// Regression test for browser-side validation of the allow-modals sandbox
+// attribute. A sandboxed frame without allow-modals should not be able to show
+// modal dialogs (alert, confirm, prompt). This is a variant of
+// WindowOpenDisallowedFromSandboxedFrame for the kModals sandbox flag.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       ModalDialogDisallowedFromSandboxedFrame) {
+  IsolateOrigin("b.com");
+
+  GURL main_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  WebContentsImpl* web_contents =
+      static_cast<WebContentsImpl*>(shell()->web_contents());
+  FrameTreeNode* root = web_contents->GetPrimaryFrameTree().root();
+  RenderFrameHostImpl* main_frame = root->current_frame_host();
+
+  // Create cross-site sandboxed child frame. The frame lacks the allow-modals
+  // attribute, so it should not be allowed to show modal dialogs.
+  GURL child_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
+  {
+    std::string js_str = base::StringPrintf(
+        "var frame = document.createElement('iframe'); "
+        "frame.sandbox = 'allow-scripts'; "
+        "frame.src = '%s'; "
+        "document.body.appendChild(frame);",
+        child_url.spec().c_str());
+    EXPECT_TRUE(ExecJs(main_frame, js_str));
+    ASSERT_TRUE(WaitForLoadStop(web_contents));
+  }
+
+  RenderFrameHostImpl* subframe = root->child_at(0)->current_frame_host();
+  EXPECT_TRUE(
+      subframe->GetSiteInstance()->GetSecurityPrincipal().IsSandboxed());
+  EXPECT_TRUE(subframe->IsSandboxed(network::mojom::WebSandboxFlags::kModals));
+
+  // Simulate that the b.com renderer is compromised and sends an IPC to show
+  // a modal dialog, bypassing renderer-side checks in LocalDOMWindow::alert().
+  // The browser process should detect this and terminate the renderer.
+  RenderProcessHostBadIpcMessageWaiter kill_waiter(subframe->GetProcess());
+  subframe->RunModalAlertDialog(u"test", false, base::DoNothing());
+  EXPECT_EQ(bad_message::RFH_MODAL_DIALOG_FROM_SANDBOXED_FRAME,
+            kill_waiter.Wait());
+  EXPECT_FALSE(subframe->IsRenderFrameLive());
+}
+
 // Regression test for browser-side validation of POST submissions in
 // CreateNewWindow. See https://crbug.com/487768779.
 IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,