From e8ef8db04d0db6d39b26c42f7a04578e31d09cd9 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Thu, 2 May 2019 13:58:18 +0200 Subject: [Backport] Fix for CVE-2019-5823 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent WindowClient.navigate() from cancelling a browser-initiated navigation. Otherwise, a service worker can prevent you from navigating where you want to go via the omnibox. Note: this is similar to WebContentsImpl::OnGoToEntryAtOffset() for renderer-initiated history navigations. Bug: 930154 Change-Id: I3a687ccc8ba4420d2369adb24f63c2702bdeeff1 Reviewed-on: https://chromium-review.googlesource.com/c/1477454 Commit-Queue: Matt Falkenhagen Commit-Queue: Arthur Sonzogni Reviewed-by: Arthur Sonzogni Auto-Submit: Matt Falkenhagen Cr-Commit-Position: refs/heads/master@{#633231} Reviewed-by: Michael Brüning --- .../service_worker/service_worker_client_utils.cc | 15 ++++++++++- .../service_worker_clients_api_browsertest.cc | 30 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) (limited to 'chromium/content/browser') diff --git a/chromium/content/browser/service_worker/service_worker_client_utils.cc b/chromium/content/browser/service_worker/service_worker_client_utils.cc index 4a597a8c505..6221fca02b7 100644 --- a/chromium/content/browser/service_worker/service_worker_client_utils.cc +++ b/chromium/content/browser/service_worker/service_worker_client_utils.cc @@ -284,8 +284,21 @@ void NavigateClientOnUI(const GURL& url, return; } - int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id(); + // Reject the navigate() call if there is an ongoing browser-initiated + // navigation. Not rejecting it would allow websites to prevent the user from + // navigating away. See https://crbug.com/930154. + NavigationRequest* ongoing_navigation_request = + rfhi->frame_tree_node()->frame_tree()->root()->navigation_request(); + if (ongoing_navigation_request && + ongoing_navigation_request->browser_initiated()) { + base::PostTaskWithTraits( + FROM_HERE, {BrowserThread::IO}, + base::BindOnce(std::move(callback), ChildProcessHost::kInvalidUniqueID, + MSG_ROUTING_NONE)); + return; + } + int frame_tree_node_id = rfhi->frame_tree_node()->frame_tree_node_id(); Navigator* navigator = rfhi->frame_tree_node()->navigator(); navigator->RequestOpenURL( rfhi, url, url::Origin::Create(script_url), false /* uses_post */, diff --git a/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc b/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc index 9710079da14..49c5279baaf 100644 --- a/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc +++ b/chromium/content/browser/service_worker/service_worker_clients_api_browsertest.cc @@ -257,6 +257,36 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, EXPECT_EQ(title, title_watcher.WaitAndGetTitle()); } +// Tests a WindowClient.navigate() call during a browser-initiated navigation. +// Regression test for https://crbug.com/930154. +IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, + NavigateDuringBrowserNavigation) { + // Load a page that registers a service worker. + EXPECT_TRUE(NavigateToURL(shell(), + embedded_test_server()->GetURL( + "/service_worker/create_service_worker.html"))); + EXPECT_EQ("DONE", EvalJs(shell(), "register('client_api_worker.js');")); + + // Load the test page. + EXPECT_TRUE(NavigateToURL( + shell(), + embedded_test_server()->GetURL("/service_worker/request_navigate.html"))); + + // Start a browser-initiated navigation. + GURL url(embedded_test_server()->GetURL("/title1.html")); + TestNavigationManager navigation(shell()->web_contents(), url); + shell()->LoadURL(url); + EXPECT_TRUE(navigation.WaitForRequestStart()); + + // Have the service worker call client.navigate() to try to go to another + // URL. It should fail. + EXPECT_EQ("navigate failed", EvalJs(shell(), "requestToNavigate();")); + + // The browser-initiated navigation should finish. + navigation.WaitForNavigationFinished(); // Resume navigation. + EXPECT_TRUE(navigation.was_successful()); +} + // Tests a successful Clients.openWindow() call. IN_PROC_BROWSER_TEST_F(ServiceWorkerClientsApiBrowserTest, OpenWindow) { ActivatedServiceWorkerObserver observer; -- cgit v1.2.1