Disallow navigation to documents not explicitly listed as web accessible.

The existing check for web accessible resources is inadequate and allows
navigations to non-whitelisted pages to succeed. This patch ensures that
the document is listed explicitly in the manifest when a navigation is
performed to it.

BUG=576867

Review-Url: https://codereview.chromium.org/2007133004
Cr-Commit-Position: refs/heads/master@{#397066}

chrome/browser/extensions/extension_protocols_unittest.cc

@@ -198,8 +198,8 @@ TEST_F(ExtensionProtocolTest, IncognitoRequest) {
   } cases[] = {
     {"spanning disabled", false, false, false, false},
     {"split disabled", true, false, false, false},
-    {"spanning enabled", false, true, false, true},
-    {"split enabled", true, true, true, true},
+    {"spanning enabled", false, true, false, false},
+    {"split enabled", true, true, true, false},
   };
 
   for (size_t i = 0; i < arraysize(cases); ++i) {
@@ -346,7 +346,9 @@ TEST_F(ExtensionProtocolTest, AllowFrameRequests) {
                                     false,
                                     false);
 
-  // All MAIN_FRAME and SUB_FRAME requests should succeed.
+  // All MAIN_FRAME requests should succeed. SUB_FRAME requests that are not
+  // explicitly listed in web_accesible_resources or same-origin to the parent
+  // should not succeed.
   {
     std::unique_ptr<net::URLRequest> request(
         resource_context_.GetRequestContext()->CreateRequest(
@@ -361,7 +363,7 @@ TEST_F(ExtensionProtocolTest, AllowFrameRequests) {
             extension->GetResourceURL("test.dat"), net::DEFAULT_PRIORITY,
             &test_delegate_));
     StartRequest(request.get(), content::RESOURCE_TYPE_SUB_FRAME);
-    EXPECT_EQ(net::URLRequestStatus::SUCCESS, request->status().status());
+    EXPECT_EQ(net::URLRequestStatus::FAILED, request->status().status());
   }
 
   // And subresource types, such as media, should fail.

chrome/browser/extensions/extension_resource_request_policy_apitest.cc

@@ -8,8 +8,12 @@
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_switches.h"
+#include "chrome/test/base/test_switches.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/common/switches.h"
 #include "net/dns/mock_host_resolver.h"
@@ -319,3 +323,59 @@ IN_PROC_BROWSER_TEST_F(ExtensionResourceRequestPolicyTest,
                        MAYBE_ExtensionAccessibleResources) {
   ASSERT_TRUE(RunExtensionSubtest("accessible_cer", "main.html")) << message_;
 }
+
+class NavigationErrorObserver : public content::WebContentsObserver {
+ public:
+  NavigationErrorObserver(content::WebContents* web_contents, const GURL& url)
+      : content::WebContentsObserver(web_contents),
+        url_(url),
+        saw_navigation_(false) {}
+
+  void DidFinishNavigation(content::NavigationHandle* handle) override {
+    if (handle->GetURL() != url_)
+      return;
+    EXPECT_TRUE(handle->IsErrorPage());
+    saw_navigation_ = true;
+    if (run_loop_.running())
+      run_loop_.Quit();
+  }
+
+  void Wait() {
+    if (!saw_navigation_)
+      run_loop_.Run();
+  }
+
+ private:
+  // The url we want to see a navigation for.
+  GURL url_;
+
+  // Have we seen the navigation for |url_| yet?
+  bool saw_navigation_;
+
+  base::RunLoop run_loop_;
+
+  DISALLOW_COPY_AND_ASSIGN(NavigationErrorObserver);
+};
+
+IN_PROC_BROWSER_TEST_F(ExtensionResourceRequestPolicyTest,
+                       IframeNavigateToInaccessible) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  ASSERT_TRUE(LoadExtension(
+      test_data_dir_.AppendASCII("extension_resource_request_policy")
+          .AppendASCII("some_accessible")));
+
+  GURL iframe_navigate_url(embedded_test_server()->GetURL(
+      "/extensions/api_test/extension_resource_request_policy/"
+      "iframe_navigate.html"));
+
+  ui_test_utils::NavigateToURL(browser(), iframe_navigate_url);
+
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  GURL private_page(
+      "chrome-extension://kegmjfcnjamahdnldjmlpachmpielcdk/private.html");
+  NavigationErrorObserver observer(web_contents, private_page);
+  ASSERT_TRUE(content::ExecuteScript(web_contents, "navigateFrameNow()"));
+  observer.Wait();
+}

chrome/test/data/extensions/api_test/extension_resource_request_policy/iframe_navigate.html

+<html>
+<body>
+<iframe id="myframe" src="chrome-extension://kegmjfcnjamahdnldjmlpachmpielcdk/public.html"></iframe>
+<script>
+function navigateFrameNow() {
+  var frame = document.getElementById("myframe");
+  myframe.src =
+    "chrome-extension://kegmjfcnjamahdnldjmlpachmpielcdk/private.html";
+}
+</script>
+</body>
+</html>

chrome/test/data/extensions/api_test/extension_resource_request_policy/some_accessible/manifest.json

+{
+  "name": "Extension with some accessible pages",
+  "version": "0.1",
+  "manifest_version": 2,
+   "key": "MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAzhxJvA02X/SxUSjnN3TchJ7GXiE21kyPwCnldsxaw7+0iTocB2NC/YSnHyYX0R0gkoxzUjhui2DsNZPcw9MGMXlGRXe5p5pL4i6ml55rhKNOiYtQ4N1ftIRa7TR3xTST8iCU+k+bpl8XfNL04c6MvQtfAn5hZ9jVLRMNLPpHUapuxct5dWokyNXlbH9SiSMT6ZQvkstq1/TQSJxZDzsHOHcuhwGwh5rug7AA0B3rgjkmAIqJHlhJWQlg4L54a7YL1Xn8iSYOj34I7uAaCKxk/tJ3jFMt+DDtZpTLCraH4doINZo8WBpqWpfs9dl4dGLDmcjHLpeQmUnrWYmGo2MOOQIDAQAB",
+  "web_accessible_resources": [
+    "public.html"
+  ]
+}

chrome/test/data/extensions/api_test/extension_resource_request_policy/some_accessible/private.html

+<html>
+<head><title>Private</title></head>
+<body>
+Private
+</body>
+</html>

chrome/test/data/extensions/api_test/extension_resource_request_policy/some_accessible/public.html

+<html>
+<head><title>Public</title></head>
+<body>
+Public
+</body>
+</html>

extensions/browser/url_request_util.cc

@@ -39,6 +39,7 @@ bool AllowCrossRendererResourceLoad(net::URLRequest* request,
   bool is_guest = WebViewRendererState::GetInstance()->GetPartitionID(
       info->GetChildID(), &partition_id);
   std::string resource_path = request->url().path();
+
   // |owner_extension == extension| needs to be checked because extension
   // resources should only be accessible to WebViews owned by that extension.
   if (is_guest && owner_extension == extension &&
@@ -48,14 +49,6 @@ bool AllowCrossRendererResourceLoad(net::URLRequest* request,
     return true;
   }
 
-  // If the request is for navigations outside of webviews, then it should be
-  // allowed. The navigation logic in CrossSiteResourceHandler will properly
-  // transfer the navigation to a privileged process before it commits.
-  if (content::IsResourceTypeFrame(info->GetResourceType()) && !is_guest) {
-    *allowed = true;
-    return true;
-  }
-
   if (!ui::PageTransitionIsWebTriggerable(info->GetPageTransition())) {
     *allowed = false;
     return true;
@@ -85,11 +78,41 @@ bool AllowCrossRendererResourceLoad(net::URLRequest* request,
     return true;
   }
 
-  // Extensions with web_accessible_resources: allow loading by regular
-  // renderers. Since not all subresources are required to be listed in a v2
-  // manifest, we must allow all loads if there are any web accessible
-  // resources. See http://crbug.com/179127.
-  if (extension->manifest_version() < 2 ||
+  DCHECK_EQ(extension->url(), request->url().GetWithEmptyPath());
+
+  // Extensions with manifest before v2 did not have web_accessible_resource
+  // section, therefore the request needs to be allowed.
+  if (extension->manifest_version() < 2) {
+    *allowed = true;
+    return true;
+  }
+
+  // Navigating the main frame to an extension URL is allowed, even if not
+  // explicitly listed as web_accessible_resource.
+  if (info->GetResourceType() == content::RESOURCE_TYPE_MAIN_FRAME) {
+    *allowed = true;
+    return true;
+  } else if (info->GetResourceType() == content::RESOURCE_TYPE_SUB_FRAME) {
+    // When navigating in subframe, allow if it is the same origin
+    // as the top-level frame. This can only be the case if the subframe
+    // request is coming from the extension process.
+    if (extension_info_map->process_map().Contains(info->GetChildID())) {
+      *allowed = true;
+      return true;
+    }
+
+    // Also allow if the file is explicitly listed as a web_accessible_resource.
+    if (WebAccessibleResourcesInfo::IsResourceWebAccessible(extension,
+                                                            resource_path)) {
+      *allowed = true;
+      return true;
+    }
+  }
+
+  // Since not all subresources are required to be listed in a v2
+  // manifest, we must allow all subresource loads if there are any web
+  // accessible resources. See http://crbug.com/179127.
+  if (!content::IsResourceTypeFrame(info->GetResourceType()) &&
       WebAccessibleResourcesInfo::HasWebAccessibleResources(extension)) {
     *allowed = true;
     return true;