Extensions: Prevent navigations to platform app frames in a tab.

Platform app resources should never load in a tab. However currently,
it's possible for browser initiated navigations to bypass that
restriction on redirects. Fix that by adding an explicit check in
ExtensionNavigationThrottle to ensure that navigations to platform app
resources only succeed in certain cases. Also, add a regression test.

BUG=1110551

Change-Id: If11f7f1f9649c8a3f7960cd51d18ef902ac272d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2347086
Auto-Submit: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799859}

chrome/browser/extensions/web_contents_browsertest.cc

@@ -24,6 +24,7 @@
 #include "content/public/test/test_navigation_observer.h"
 #include "extensions/browser/extension_api_frame_id_map.h"
 #include "extensions/browser/extension_navigation_ui_data.h"
+#include "extensions/common/manifest_handlers/background_info.h"
 
 namespace extensions {
 namespace {
@@ -98,6 +99,29 @@ IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, WebContents) {
   EXPECT_TRUE(result);
 }
 
+// Ensure that platform app frames can't be loaded in a tab even on a redirect.
+// Regression test for crbug.com/1110551.
+IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, TabNavigationToPlatformApp) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  const Extension* extension = LoadExtension(
+      test_data_dir_.AppendASCII("platform_apps").AppendASCII("minimal"));
+  ASSERT_TRUE(extension);
+
+  const GURL test_cases[] = {extension->GetResourceURL("main.html"),
+                             BackgroundInfo::GetBackgroundURL(extension)};
+  for (const GURL& app_url : test_cases) {
+    GURL redirect_to_platform_app =
+        embedded_test_server()->GetURL("/server-redirect?" + app_url.spec());
+    content::WebContents* web_contents = GetActiveWebContents(browser());
+    content::TestNavigationObserver observer(web_contents,
+                                             net::ERR_BLOCKED_BY_CLIENT);
+    ui_test_utils::NavigateToURL(browser(), redirect_to_platform_app);
+    observer.Wait();
+    EXPECT_FALSE(observer.last_navigation_succeeded());
+  }
+}
+
 // Test that we correctly set up the ExtensionNavigationUIData for each
 // navigation.
 IN_PROC_BROWSER_TEST_F(ExtensionBrowserTest, ExtensionNavigationUIData) {

chrome/browser/ui/browser_navigator.cc

@@ -68,9 +68,7 @@
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 #include "chrome/browser/apps/app_service/launch_utils.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
-#include "extensions/browser/extension_registry.h"
 #include "extensions/common/extension.h"
-#include "extensions/common/extension_set.h"
 #endif
 
 using content::GlobalRequestID;
@@ -500,16 +498,6 @@ void Navigate(NavigateParams* params) {
   if (!AdjustNavigateParamsForURL(params))
     return;
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-  const extensions::Extension* extension =
-      extensions::ExtensionRegistry::Get(params->initiating_profile)
-          ->enabled_extensions()
-          .GetExtensionOrAppByURL(params->url);
-  // Platform apps cannot navigate. Block the request.
-  if (extension && extension->is_platform_app())
-    params->url = GURL(chrome::kExtensionInvalidRequestURL);
-#endif
-
   // Trying to open a background tab when in an app browser results in
   // focusing a regular browser window an opening a tab in the background
   // of that window. Change the disposition to NEW_FOREGROUND_TAB so that

extensions/browser/extension_navigation_throttle.cc

@@ -13,10 +13,14 @@
 #include "content/public/browser/storage_partition_config.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/common/url_constants.h"
+#include "extensions/browser/app_window/app_window.h"
+#include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_browser_client.h"
+#include "extensions/browser/guest_view/app_view/app_view_guest.h"
 #include "extensions/browser/guest_view/web_view/web_view_guest.h"
 #include "extensions/browser/url_request_util.h"
+#include "extensions/browser/view_type_utils.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/common/extension_set.h"
@@ -30,6 +34,66 @@
 
 namespace extensions {
 
+namespace {
+
+// Whether a navigation to the |platform_app| resource should be blocked in the
+// given |web_contents|.
+bool ShouldBlockNavigationToPlatformAppResource(
+    const Extension* platform_app,
+    content::WebContents* web_contents) {
+  ViewType view_type = GetViewType(web_contents);
+  DCHECK_NE(VIEW_TYPE_INVALID, view_type);
+
+  // Navigation to platform app's background page.
+  if (view_type == VIEW_TYPE_EXTENSION_BACKGROUND_PAGE)
+    return false;
+
+  // Navigation within an extension dialog, e.g. this is used by ChromeOS file
+  // manager.
+  if (view_type == VIEW_TYPE_EXTENSION_DIALOG)
+    return false;
+
+  // Navigation within an app window. The app window must belong to the
+  // |platform_app|.
+  if (view_type == VIEW_TYPE_APP_WINDOW) {
+    AppWindowRegistry* registry =
+        AppWindowRegistry::Get(web_contents->GetBrowserContext());
+    DCHECK(registry);
+    AppWindow* app_window = registry->GetAppWindowForWebContents(web_contents);
+    DCHECK(app_window);
+    return app_window->extension_id() != platform_app->id();
+  }
+
+  // Navigation within a guest web contents.
+  if (view_type == VIEW_TYPE_EXTENSION_GUEST) {
+    // Platform apps can be embedded by other platform apps using an <appview>
+    // tag.
+    AppViewGuest* app_view = AppViewGuest::FromWebContents(web_contents);
+    if (app_view)
+      return false;
+
+    // Webviews owned by the platform app can embed platform app resources via
+    // "accessible_resources".
+    WebViewGuest* web_view_guest = WebViewGuest::FromWebContents(web_contents);
+    if (web_view_guest)
+      return web_view_guest->owner_host() != platform_app->id();
+
+    // Otherwise, it's a guest view that's neither a webview nor an appview
+    // (such as an extensionoptions view). Disallow.
+    return true;
+  }
+
+  DCHECK(view_type == VIEW_TYPE_BACKGROUND_CONTENTS ||
+         view_type == VIEW_TYPE_COMPONENT ||
+         view_type == VIEW_TYPE_EXTENSION_POPUP ||
+         view_type == VIEW_TYPE_TAB_CONTENTS)
+      << "Unhandled view type: " << view_type;
+
+  return true;
+}
+
+}  // namespace
+
 ExtensionNavigationThrottle::ExtensionNavigationThrottle(
     content::NavigationHandle* navigation_handle)
     : content::NavigationThrottle(navigation_handle) {}
@@ -139,6 +203,14 @@ ExtensionNavigationThrottle::WillStartOrRedirectRequest() {
     }
   }
 
+  if (target_extension->is_platform_app() &&
+      ShouldBlockNavigationToPlatformAppResource(target_extension,
+                                                 web_contents)) {
+    RecordExtensionResourceAccessResult(
+        source_id, url, ExtensionResourceAccessResult::kFailure);
+    return content::NavigationThrottle::BLOCK_REQUEST;
+  }
+
   // Navigations with no initiator (e.g. browser-initiated requests) are always
   // considered trusted, and thus allowed.
   //
@@ -192,6 +264,7 @@ ExtensionNavigationThrottle::WillStartOrRedirectRequest() {
   // Content Security Policy. But CSP is incapable of blocking the
   // chrome-extension scheme. Thus, this case must be handled specially
   // here.
+  // TODO(karandeepb): Investigate if this check can be removed.
   if (target_extension->is_platform_app()) {
     RecordExtensionResourceAccessResult(source_id, url,
                                         ExtensionResourceAccessResult::kCancel);