From 3a40d93884129f66a868ce157eaa75f55a3d876f Mon Sep 17 00:00:00 2001
From: greengrape <greengrape@google.com>
Date: Tue, 28 Jul 2026 09:13:56 -0700
Subject: [PATCH] Fix cross-IWA navigation restrictions for popups

The NavigationCapturingProcess previously returned early if capturing was
disabled due to dispositions like WindowOpenDisposition::NEW_POPUP,
bypassing cross-Isolated Web App limits. This allowed an installed IWA
to bypass cross-IWA navigation restrictions and spoof another app via
window.open() with the 'popup' feature.

This patch updates HandleIsolatedWebAppNavigation() to execute the
restriction checks for cross-IWA window target popups independently of 
the capture logic.

Bug: 532931962
TAG=agy

Change-Id: I1eea864f1008f517e1f58217f02f2e1c6a6a6964
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8158641
Reviewed-by: Dibyajyoti Pal <dibyapal@chromium.org>
Reviewed-by: Vlad Krot <vkrot@google.com>
Commit-Queue: Andrew Rayskiy <greengrape@google.com>
Cr-Commit-Position: refs/heads/main@{#1669557}
---
diff --git a/chrome/browser/ui/web_applications/navigation_capturing_process.cc b/chrome/browser/ui/web_applications/navigation_capturing_process.cc
index 3a198d88..1d19fab 100644
--- a/chrome/browser/ui/web_applications/navigation_capturing_process.cc
+++ b/chrome/browser/ui/web_applications/navigation_capturing_process.cc
@@ -280,6 +280,35 @@
                                                   base::DoNothing());
 }
 
+bool IsCrossIwaNavigation(
+    const NavigateParams& params,
+    const webapps::AppId& iwa_id,
+    const std::optional<webapps::AppId>& source_browser_app_id,
+    const std::optional<webapps::AppId>& parent_app_id) {
+  // Service worker `clients.openWindow()` arrives with no source browser and a
+  // non-link transition, so the link-based source check below does not apply.
+  // Use the initiator origin to enforce the same cross-IWA restriction.
+  if (params.is_service_worker_open_window && params.initiator_origin &&
+      !params.initiator_origin->IsSameOriginWith(params.url)) {
+    return true;
+  }
+
+  // Any links: same-IWA or cross-IWA window.open(), same-IWA or cross-IWA
+  // anchor link, cross-IWA meta tag redirect. Cancel navigations that do not
+  // originate from a browser for the target app (or its parent app), regardless
+  // of disposition, before falling through to the disposition-specific handling
+  // below.
+  if (ui::PageTransitionCoreTypeIs(params.transition,
+                                   ui::PAGE_TRANSITION_LINK) &&
+      source_browser_app_id != iwa_id &&
+      (!parent_app_id.has_value() ||
+       source_browser_app_id != parent_app_id.value())) {
+    return true;
+  }
+
+  return false;
+}
+
 }  // namespace
 
 NavigationCapturingOverride::~NavigationCapturingOverride() = default;
@@ -893,6 +922,13 @@
   const webapps::AppId& iwa_id = *first_navigation_app_id_;
   const DisplayMode& app_display_mode = *first_navigation_app_display_mode_;
 
+  if (IsCrossIwaNavigation(params, iwa_id, source_browser_app_id_,
+                           first_navigation_parent_app_id_)) {
+    // TODO(crbug.com/424422466): Support cross-IWA navigations to start_url.
+    return CancelInitialNavigation(
+        NavigationCapturingInitialResult::kNavigationCanceled);
+  }
+
   // Prefer `params.browser` if it's a compatible IWA browser.
   bool iwa_browser =
       params.browser &&
@@ -934,34 +970,13 @@
     return CapturingDisabled();
   }
 
-  // Service worker `clients.openWindow()` arrives with no source browser and a
-  // non-link transition, so the link-based source check below does not apply.
-  // Use the initiator origin to enforce the same cross-IWA restriction.
-  if (params.is_service_worker_open_window && params.initiator_origin &&
-      !params.initiator_origin->IsSameOriginWith(params.url)) {
-    // TODO(crbug.com/424422466): Support cross-IWA navigations to start_url.
-    return CancelInitialNavigation(
-        NavigationCapturingInitialResult::kNavigationCanceled);
-  }
-
   if (ui::PageTransitionCoreTypeIs(params.transition,
-                                   ui::PAGE_TRANSITION_LINK)) {
-    // Any links: same-IWA or cross-IWA window.open(), same-IWA or cross-IWA
-    // anchor link, cross-IWA meta tag redirect.
-    if (source_browser_app_id_ != iwa_id &&
-        (!first_navigation_parent_app_id_.has_value() ||
-         source_browser_app_id_ != first_navigation_parent_app_id_.value())) {
-      // TODO(crbug.com/424422466): Support cross-IWA navigations to start_url.
-      return CancelInitialNavigation(
-          NavigationCapturingInitialResult::kNavigationCanceled);
-    }
-
-    if (IsAuxiliaryBrowsingContext(params)) {
-      debug_data_.Set("is_auxiliary_browsing_context", true);
-      Browser* aux_window =
-          CreateWebAppWindowFromNavigationParams(iwa_id, params);
-      return AuxiliaryContextInAppWindow(aux_window);
-    }
+                                   ui::PAGE_TRANSITION_LINK) &&
+      IsAuxiliaryBrowsingContext(params)) {
+    debug_data_.Set("is_auxiliary_browsing_context", true);
+    Browser* aux_window =
+        CreateWebAppWindowFromNavigationParams(iwa_id, params);
+    return AuxiliaryContextInAppWindow(aux_window);
   }
 
   // Auxiliary browsing contexts should only be openable via link transitions.
diff --git a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
index 88c2180..64b8ff2 100644
--- a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
+++ b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
@@ -13,6 +13,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
+#include "base/test/bind.h"
 #include "base/test/gmock_expected_support.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/run_until.h"
@@ -72,6 +73,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/service_worker_test_helpers.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/test/result_catcher.h"
 #include "net/base/net_errors.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -1838,6 +1840,51 @@
   EXPECT_EQ(browsers_before, GlobalBrowserCollection::GetInstance()->GetSize());
 }
 
+IN_PROC_BROWSER_TEST_P(IsolatedWebAppLaunchHandlingBrowserTest,
+                       CrossOriginWindowOpenPopup) {
+  std::unique_ptr<ScopedBundledIsolatedWebApp> source_app =
+      IsolatedWebAppBuilder(ManifestBuilder()).BuildBundle();
+  ASSERT_OK_AND_ASSIGN(IsolatedWebAppUrlInfo source_url_info,
+                       source_app->Install(profile()));
+
+  std::unique_ptr<ScopedBundledIsolatedWebApp> target_app =
+      IsolatedWebAppBuilder(
+          ManifestBuilder().SetLaunchHandlerClientMode(GetParam()))
+          .AddHtml("/something/weird.html", "meow")
+          .BuildBundle();
+  ASSERT_OK_AND_ASSIGN(IsolatedWebAppUrlInfo target_url_info,
+                       target_app->Install(profile()));
+
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(
+          OpenIsolatedWebApp(profile(), source_url_info.app_id()));
+
+  const size_t browsers_before =
+      GlobalBrowserCollection::GetInstance()->GetSize();
+
+  const GURL target_url =
+      target_url_info.origin().GetURL().Resolve("/something/weird.html");
+
+  std::unique_ptr<content::WebContentsDestroyedWatcher> destroyed_watcher;
+  base::CallbackListSubscription creation_subscription =
+      content::RegisterWebContentsCreationCallback(
+          base::BindLambdaForTesting([&](content::WebContents* wc) {
+            destroyed_watcher =
+                std::make_unique<content::WebContentsDestroyedWatcher>(wc);
+          }));
+  ASSERT_TRUE(content::ExecJs(
+      web_contents,
+      content::JsReplace("window.open($1, '_blank', 'popup')", target_url)));
+  ASSERT_TRUE(destroyed_watcher);
+  destroyed_watcher->Wait();
+
+  // The cross-origin `window.open()` popup must not open a window for the
+  // target app.
+  EXPECT_FALSE(AppBrowserController::FindForWebApp(*profile(),
+                                                   target_url_info.app_id()));
+  EXPECT_EQ(browsers_before, GlobalBrowserCollection::GetInstance()->GetSize());
+}
+
 IN_PROC_BROWSER_TEST_P(IsolatedWebAppLaunchHandlingBrowserTest, PlainLaunch) {
   std::unique_ptr<ScopedBundledIsolatedWebApp> app =
       IsolatedWebAppBuilder(

diff --git a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
index 88c2180..64b8ff2 100644
--- a/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
+++ b/chrome/browser/web_applications/isolated_web_apps/isolated_web_app_browsertest.cc
@@ -13,6 +13,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/run_loop.h"
 #include "base/strings/string_util.h"
+#include "base/test/bind.h"
 #include "base/test/gmock_expected_support.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/test/run_until.h"
@@ -72,6 +73,7 @@
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/service_worker_test_helpers.h"
 #include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
 #include "extensions/test/result_catcher.h"
 #include "net/base/net_errors.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -1838,6 +1840,51 @@
   EXPECT_EQ(browsers_before, GlobalBrowserCollection::GetInstance()->GetSize());
 }
 
+IN_PROC_BROWSER_TEST_P(IsolatedWebAppLaunchHandlingBrowserTest,
+                       CrossOriginWindowOpenPopup) {
+  std::unique_ptr<ScopedBundledIsolatedWebApp> source_app =
+      IsolatedWebAppBuilder(ManifestBuilder()).BuildBundle();
+  ASSERT_OK_AND_ASSIGN(IsolatedWebAppUrlInfo source_url_info,
+                       source_app->Install(profile()));
+
+  std::unique_ptr<ScopedBundledIsolatedWebApp> target_app =
+      IsolatedWebAppBuilder(
+          ManifestBuilder().SetLaunchHandlerClientMode(GetParam()))
+          .AddHtml("/something/weird.html", "meow")
+          .BuildBundle();
+  ASSERT_OK_AND_ASSIGN(IsolatedWebAppUrlInfo target_url_info,
+                       target_app->Install(profile()));
+
+  content::WebContents* web_contents =
+      content::WebContents::FromRenderFrameHost(
+          OpenIsolatedWebApp(profile(), source_url_info.app_id()));
+
+  const size_t browsers_before =
+      GlobalBrowserCollection::GetInstance()->GetSize();
+
+  const GURL target_url =
+      target_url_info.origin().GetURL().Resolve("/something/weird.html");
+
+  std::unique_ptr<content::WebContentsDestroyedWatcher> destroyed_watcher;
+  base::CallbackListSubscription creation_subscription =
+      content::RegisterWebContentsCreationCallback(
+          base::BindLambdaForTesting([&](content::WebContents* wc) {
+            destroyed_watcher =
+                std::make_unique<content::WebContentsDestroyedWatcher>(wc);
+          }));
+  ASSERT_TRUE(content::ExecJs(
+      web_contents,
+      content::JsReplace("window.open($1, '_blank', 'popup')", target_url)));
+  ASSERT_TRUE(destroyed_watcher);
+  destroyed_watcher->Wait();
+
+  // The cross-origin `window.open()` popup must not open a window for the
+  // target app.
+  EXPECT_FALSE(AppBrowserController::FindForWebApp(*profile(),
+                                                   target_url_info.app_id()));
+  EXPECT_EQ(browsers_before, GlobalBrowserCollection::GetInstance()->GetSize());
+}
+
 IN_PROC_BROWSER_TEST_P(IsolatedWebAppLaunchHandlingBrowserTest, PlainLaunch) {
   std::unique_ptr<ScopedBundledIsolatedWebApp> app =
       IsolatedWebAppBuilder(