diff --git a/chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc b/chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc index a924dd7d66269..ebf32a400e4d7 100644 --- a/chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc +++ b/chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc @@ -61,7 +61,11 @@ #include "components/tabs/public/tab_interface.h" #include "components/url_formatter/url_formatter.h" #include "components/webapps/browser/install_result_code.h" +#include "content/public/browser/navigation_controller.h" +#include "content/public/browser/render_frame_host.h" +#include "content/public/browser/render_process_host.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/referrer.h" #include "content/public/common/content_features.h" #include "content/public/test/background_color_change_waiter.h" #include "content/public/test/browser_test.h" @@ -780,6 +784,75 @@ IN_PROC_BROWSER_TEST_P(WebAppTabStripBrowserTest, TargetBlankLink) { "/web_apps/tab_strip_customizations.html#target_blank")); } +// Regression test: TabbedWebAppNavigationThrottle::WillStartRequest calls +// web_contents->ClosePage() on its own WebContents and then continues to use +// `this` (via FocusHomeTab -> navigation_handle()). If ClosePage() takes its +// synchronous branch (which happens when the current RenderFrameHost is not +// live, e.g. its renderer process has not been initialized or has crashed), +// the WebContents — and with it the NavigationRequest, the throttle registry, +// and the throttle itself — is destroyed synchronously before FocusHomeTab() +// runs, leading to a heap-use-after-free in the browser process. +// +// This models a compromised renderer (or a sad-tab) inside a tabbed PWA +// window: a fresh blank tab whose renderer is not live is navigated to a +// home-tab-scope URL. +IN_PROC_BROWSER_TEST_P(WebAppTabStripBrowserTest, + ClosePageDuringThrottleSyncFree) { + GURL start_url = + embedded_test_server()->GetURL("/web_apps/tab_strip_customizations.html"); + webapps::AppId app_id = InstallTestWebApp(start_url); + Browser* app_browser = FindWebAppBrowser(browser()->profile(), app_id); + TabStripModel* tab_strip = app_browser->tab_strip_model(); + + ASSERT_TRUE(registrar().IsTabbedWindowModeEnabled(app_id)); + ASSERT_TRUE(tab_strip->IsTabPinned(0)); + + // Create a fresh blank WebContents and add it to the app browser's tab + // strip. TabModel attaches tab helpers (including WebAppTabHelper) on + // insertion. Because we never navigate this tab before the test navigation + // below, its RenderFrame is never created, so IsRenderFrameLive() is false + // and GetLastCommittedURL() is invalid. + content::WebContents::CreateParams create_params(app_browser->profile()); + std::unique_ptr blank_contents = + content::WebContents::Create(create_params); + content::WebContents* blank_tab = blank_contents.get(); + tab_strip->AppendWebContents(std::move(blank_contents), /*foreground=*/true); + + ASSERT_EQ(tab_strip->count(), 2); + ASSERT_FALSE(blank_tab->GetLastCommittedURL().is_valid()); + + // If something above ended up creating the RenderFrame, terminate the + // renderer so that RenderFrameHostImpl::ClosePage() takes its synchronous + // (!IsRenderFrameLive()) branch. + if (blank_tab->GetPrimaryMainFrame()->IsRenderFrameLive()) { + content::RenderProcessHost* rph = + blank_tab->GetPrimaryMainFrame()->GetProcess(); + content::RenderProcessHostWatcher watcher( + rph, content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); + rph->Shutdown(0); + watcher.Wait(); + } + ASSERT_FALSE(blank_tab->GetPrimaryMainFrame()->IsRenderFrameLive()); + + // Browser-initiated navigation of the blank tab to a home-tab-scope URL. + // TabbedWebAppNavigationThrottle::WillStartRequest will see: + // !navigating_from_home_tab && navigation_url_is_home_url + // && count() > 1 && !GetLastCommittedURL().is_valid() + // and call web_contents->ClosePage(). Because the renderer is not live, + // ClosePage() synchronously destroys the WebContents (and the throttle). + // The throttle then calls FocusHomeTab(), which dereferences the freed + // `this->registry_` and the freed NavigationRequest. + // + // On an ASAN build this triggers heap-use-after-free in + // content::NavigationThrottle::navigation_handle() / + // OpenURLParams::FromNavigationHandle(). + blank_tab->GetController().LoadURL(start_url, content::Referrer(), + ui::PAGE_TRANSITION_LINK, std::string()); + + // If we get here without crashing, the bug is fixed. + base::RunLoop().RunUntilIdle(); +} + IN_PROC_BROWSER_TEST_P(WebAppTabStripBrowserTest, OpenInChrome) { GURL start_url = embedded_test_server()->GetURL("/web_apps/tab_strip_customizations.html");