This patch implements a mechanism for more granular link URL permissions (filtering on scheme/host). This fixes the bug that allowed PDFs to have working links to any "chrome://" URLs.

BUG=528505,226927

Review URL: https://codereview.chromium.org/1362433002

Cr-Commit-Position: refs/heads/master@{#351705}
diff --git a/chrome/browser/extensions/chrome_extension_web_contents_observer.cc b/chrome/browser/extensions/chrome_extension_web_contents_observer.cc
index b692dfb..52e2126 100644
--- a/chrome/browser/extensions/chrome_extension_web_contents_observer.cc
+++ b/chrome/browser/extensions/chrome_extension_web_contents_observer.cc
@@ -8,7 +8,9 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/window_controller.h"
 #include "chrome/common/extensions/chrome_extension_messages.h"
+#include "chrome/common/url_constants.h"
 #include "content/public/browser/browser_context.h"
+#include "content/public/browser/child_process_security_policy.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_view_host.h"
@@ -34,6 +36,35 @@
     content::RenderViewHost* render_view_host) {
   ReloadIfTerminated(render_view_host);
   ExtensionWebContentsObserver::RenderViewCreated(render_view_host);
+
+  const Extension* extension = GetExtension(render_view_host);
+  if (!extension)
+    return;
+
+  int process_id = render_view_host->GetProcess()->GetID();
+  auto policy = content::ChildProcessSecurityPolicy::GetInstance();
+
+  // Components of chrome that are implemented as extensions or platform apps
+  // are allowed to use chrome://resources/ URLs.
+  if ((extension->is_extension() || extension->is_platform_app()) &&
+      Manifest::IsComponentLocation(extension->location())) {
+    policy->GrantOrigin(process_id,
+                        url::Origin(GURL(content::kChromeUIResourcesURL)));
+  }
+
+  // Extensions, legacy packaged apps, and component platform apps are allowed
+  // to use chrome://favicon/ and chrome://extension-icon/ URLs. Hosted apps are
+  // not allowed because they are served via web servers (and are generally
+  // never given access to Chrome APIs).
+  if (extension->is_extension() ||
+      extension->is_legacy_packaged_app() ||
+      (extension->is_platform_app() &&
+       Manifest::IsComponentLocation(extension->location()))) {
+    policy->GrantOrigin(process_id,
+                        url::Origin(GURL(chrome::kChromeUIFaviconURL)));
+    policy->GrantOrigin(process_id,
+                        url::Origin(GURL(chrome::kChromeUIExtensionIconURL)));
+  }
 }
 
 bool ChromeExtensionWebContentsObserver::OnMessageReceived(
diff --git a/chrome/browser/pdf/pdf_extension_test.cc b/chrome/browser/pdf/pdf_extension_test.cc
index 84101eb..1aebcde 100644
--- a/chrome/browser/pdf/pdf_extension_test.cc
+++ b/chrome/browser/pdf/pdf_extension_test.cc
@@ -32,6 +32,7 @@
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/plugin_service.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/browser_test_utils.h"
 #include "extensions/browser/extension_registry.h"
@@ -100,14 +101,7 @@
     // being seen due to the BrowserPluginGuest not being available yet (see
     // crbug.com/498077). So instead use |LoadPdf| which ensures that the PDF is
     // loaded before continuing.
-    ASSERT_TRUE(LoadPdf(url));
-
-    content::WebContents* contents =
-        browser()->tab_strip_model()->GetActiveWebContents();
-    content::BrowserPluginGuestManager* guest_manager =
-        contents->GetBrowserContext()->GetGuestManager();
-    content::WebContents* guest_contents =
-        guest_manager->GetFullPageGuest(contents);
+    content::WebContents* guest_contents = LoadPdfGetGuestContents(url);
     ASSERT_TRUE(guest_contents);
 
     base::FilePath test_data_dir;
@@ -140,6 +134,21 @@
     return pdf_extension_test_util::EnsurePDFHasLoaded(web_contents);
   }
 
+  // Same as |LoadPdf|, but also returns a pointer to the guest WebContents for
+  // the loaded PDF. Returns nullptr if the load fails.
+  content::WebContents* LoadPdfGetGuestContents(const GURL& url) {
+    if (!LoadPdf(url))
+      return nullptr;
+
+    content::WebContents* contents =
+        browser()->tab_strip_model()->GetActiveWebContents();
+    content::BrowserPluginGuestManager* guest_manager =
+        contents->GetBrowserContext()->GetGuestManager();
+    content::WebContents* guest_contents =
+        guest_manager->GetFullPageGuest(contents);
+    return guest_contents;
+  }
+
   // Load all the PDFs contained in chrome/test/data/<dir_name>. This only runs
   // the test if base::Hash(filename) mod kNumberLoadTestParts == k in order
   // to shard the files evenly across values of k in [0, kNumberLoadTestParts).
@@ -382,6 +391,27 @@
                            true);
 }
 
+// This test ensures that link permissions are enforced properly in PDFs.
+IN_PROC_BROWSER_TEST_F(PDFExtensionTest, LinkPermissions) {
+  GURL test_pdf_url(embedded_test_server()->GetURL("/pdf/test.pdf"));
+  content::WebContents* guest_contents = LoadPdfGetGuestContents(test_pdf_url);
+  ASSERT_TRUE(guest_contents);
+
+  // chrome://favicon links should be allowed for PDFs, while chrome://settings
+  // links should not.
+  GURL valid_link_url("chrome://favicon/https://www.google.ca/");
+  GURL invalid_link_url("chrome://settings");
+
+  GURL unfiltered_valid_link_url(valid_link_url);
+  content::RenderProcessHost* rph = guest_contents->GetRenderProcessHost();
+  rph->FilterURL(true, &valid_link_url);
+  rph->FilterURL(true, &invalid_link_url);
+
+  // Invalid link URLs should be changed to "about:blank" when filtered.
+  EXPECT_EQ(unfiltered_valid_link_url, valid_link_url);
+  EXPECT_EQ(GURL("about:blank"), invalid_link_url);
+}
+
 class MaterialPDFExtensionTest : public PDFExtensionTest {
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(switches::kEnablePdfMaterialUI);
diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index b748a72..9e94f34 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -4,6 +4,8 @@
 
 #include "content/browser/child_process_security_policy_impl.h"
 
+#include <utility>
+
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
@@ -87,6 +89,11 @@
                          file_permissions_.size());
   }
 
+  // Grant permission to request URLs with the specified origin.
+  void GrantOrigin(const url::Origin& origin) {
+    origin_set_.insert(origin);
+  }
+
   // Grant permission to request URLs with the specified scheme.
   void GrantScheme(const std::string& scheme) {
     scheme_policy_[scheme] = true;
@@ -168,10 +175,15 @@
 
   // Determine whether permission has been granted to commit |url|.
   bool CanCommitURL(const GURL& url) {
-    // Having permission to a scheme implies permssion to all of its URLs.
-    SchemeMap::const_iterator judgment(scheme_policy_.find(url.scheme()));
-    if (judgment != scheme_policy_.end())
-      return judgment->second;
+    // Having permission to a scheme implies permission to all of its URLs.
+    SchemeMap::const_iterator scheme_judgment(
+        scheme_policy_.find(url.scheme()));
+    if (scheme_judgment != scheme_policy_.end())
+      return scheme_judgment->second;
+
+    // Otherwise, check for permission for specific origin.
+    if (ContainsKey(origin_set_, url::Origin(url)))
+      return true;
 
     // file:// URLs are more granular.  The child may have been given
     // permission to a specific file but not the file:// scheme in general.
@@ -242,6 +254,7 @@
 
  private:
   typedef std::map<std::string, bool> SchemeMap;
+  typedef std::set<url::Origin> OriginSet;
 
   typedef int FilePermissionFlags;  // bit-set of base::File::Flags
   typedef std::map<base::FilePath, FilePermissionFlags> FileMap;
@@ -255,6 +268,10 @@
   // or revoked.
   SchemeMap scheme_policy_;
 
+  // The set of URL origins to which the child process has been granted
+  // permission.
+  OriginSet origin_set_;
+
   // The set of files the child process is permited to upload to the web.
   FileMap file_permissions_;
 
@@ -503,6 +520,17 @@
   state->second->GrantPermissionForMidiSysEx();
 }
 
+void ChildProcessSecurityPolicyImpl::GrantOrigin(int child_id,
+                                                 const url::Origin& origin) {
+  base::AutoLock lock(lock_);
+
+  SecurityStateMap::iterator state = security_state_.find(child_id);
+  if (state == security_state_.end())
+    return;
+
+  state->second->GrantOrigin(origin);
+}
+
 void ChildProcessSecurityPolicyImpl::GrantScheme(int child_id,
                                                  const std::string& scheme) {
   base::AutoLock lock(lock_);
diff --git a/content/browser/child_process_security_policy_impl.h b/content/browser/child_process_security_policy_impl.h
index 309b667..cc5a699 100644
--- a/content/browser/child_process_security_policy_impl.h
+++ b/content/browser/child_process_security_policy_impl.h
@@ -60,6 +60,7 @@
                                const std::string& filesystem_id) override;
   void GrantDeleteFromFileSystem(int child_id,
                                  const std::string& filesystem_id) override;
+  void GrantOrigin(int child_id, const url::Origin& origin) override;
   void GrantScheme(int child_id, const std::string& scheme) override;
   bool CanReadFile(int child_id, const base::FilePath& file) override;
   bool CanCreateReadWriteFile(int child_id,
diff --git a/content/browser/child_process_security_policy_unittest.cc b/content/browser/child_process_security_policy_unittest.cc
index beb85b69..6cba102 100644
--- a/content/browser/child_process_security_policy_unittest.cc
+++ b/content/browser/child_process_security_policy_unittest.cc
@@ -16,6 +16,7 @@
 #include "storage/common/fileapi/file_system_types.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace content {
 namespace {
@@ -697,4 +698,44 @@
   EXPECT_FALSE(p->HasWebUIBindings(kRendererID));
 }
 
+// Test the granting of origin permissions, and their interactions with
+// granting scheme permissions.
+TEST_F(ChildProcessSecurityPolicyTest, OriginGranting) {
+  ChildProcessSecurityPolicyImpl* p =
+      ChildProcessSecurityPolicyImpl::GetInstance();
+
+  p->Add(kRendererID);
+
+  GURL url_foo1("chrome://foo/resource1");
+  GURL url_foo2("chrome://foo/resource2");
+  GURL url_bar("chrome://bar/resource3");
+
+  EXPECT_FALSE(p->CanRequestURL(kRendererID, url_foo1));
+  EXPECT_FALSE(p->CanRequestURL(kRendererID, url_foo2));
+  EXPECT_FALSE(p->CanRequestURL(kRendererID, url_bar));
+  EXPECT_FALSE(p->CanCommitURL(kRendererID, url_foo1));
+  EXPECT_FALSE(p->CanCommitURL(kRendererID, url_foo2));
+  EXPECT_FALSE(p->CanCommitURL(kRendererID, url_bar));
+
+  p->GrantOrigin(kRendererID, url::Origin(url_foo1));
+
+  EXPECT_TRUE(p->CanRequestURL(kRendererID, url_foo1));
+  EXPECT_TRUE(p->CanRequestURL(kRendererID, url_foo2));
+  EXPECT_FALSE(p->CanRequestURL(kRendererID, url_bar));
+  EXPECT_TRUE(p->CanCommitURL(kRendererID, url_foo1));
+  EXPECT_TRUE(p->CanCommitURL(kRendererID, url_foo2));
+  EXPECT_FALSE(p->CanCommitURL(kRendererID, url_bar));
+
+  p->GrantScheme(kRendererID, kChromeUIScheme);
+
+  EXPECT_TRUE(p->CanRequestURL(kRendererID, url_foo1));
+  EXPECT_TRUE(p->CanRequestURL(kRendererID, url_foo2));
+  EXPECT_TRUE(p->CanRequestURL(kRendererID, url_bar));
+  EXPECT_TRUE(p->CanCommitURL(kRendererID, url_foo1));
+  EXPECT_TRUE(p->CanCommitURL(kRendererID, url_foo2));
+  EXPECT_TRUE(p->CanCommitURL(kRendererID, url_bar));
+
+  p->Remove(kRendererID);
+}
+
 }  // namespace content
diff --git a/content/public/browser/child_process_security_policy.h b/content/public/browser/child_process_security_policy.h
index 06f3035..c1e6bb5 100644
--- a/content/public/browser/child_process_security_policy.h
+++ b/content/public/browser/child_process_security_policy.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "content/common/content_export.h"
 #include "url/gurl.h"
+#include "url/origin.h"
 
 namespace base {
 class FilePath;
@@ -127,6 +128,10 @@
   virtual void GrantDeleteFromFileSystem(int child_id,
                                          const std::string& filesystem_id) = 0;
 
+  // Grants the child process the capability to access URLs with the provided
+  // origin.
+  virtual void GrantOrigin(int child_id, const url::Origin& origin) = 0;
+
   // Grants the child process the capability to access URLs of the provided
   // scheme.
   virtual void GrantScheme(int child_id, const std::string& scheme) = 0;
diff --git a/content/public/common/url_constants.cc b/content/public/common/url_constants.cc
index 475b6db..ee7d0c5 100644
--- a/content/public/common/url_constants.cc
+++ b/content/public/common/url_constants.cc
@@ -55,6 +55,7 @@
 const char kUnreachableWebDataURL[] = "data:text/html,chromewebdata";
 
 const char kChromeUINetworkViewCacheURL[] = "chrome://view-http-cache/";
+const char kChromeUIResourcesURL[] = "chrome://resources/";
 const char kChromeUIShorthangURL[] = "chrome://shorthang";
 
 // This URL is loaded when a page is swapped out and replaced by a page in a
diff --git a/content/public/common/url_constants.h b/content/public/common/url_constants.h
index 9633352..f20c3e3 100644
--- a/content/public/common/url_constants.h
+++ b/content/public/common/url_constants.h
@@ -63,6 +63,7 @@
 
 // Full about URLs (including schemes).
 CONTENT_EXPORT extern const char kChromeUINetworkViewCacheURL[];
+CONTENT_EXPORT extern const char kChromeUIResourcesURL[];
 CONTENT_EXPORT extern const char kChromeUIShorthangURL[];
 
 // Special URL used to swap out a view being rendered by another process.
diff --git a/extensions/browser/extension_web_contents_observer.cc b/extensions/browser/extension_web_contents_observer.cc
index 187cfb2..fbfb5a5c 100644
--- a/extensions/browser/extension_web_contents_observer.cc
+++ b/extensions/browser/extension_web_contents_observer.cc
@@ -75,19 +75,7 @@
   if (!extension)
     return;
 
-  content::RenderProcessHost* process = render_view_host->GetProcess();
-
-  // Some extensions use chrome:// URLs.
-  // This is a temporary solution. Replace it with access to chrome-static://
-  // once it is implemented. See: crbug.com/226927.
   Manifest::Type type = extension->GetType();
-  if (type == Manifest::TYPE_EXTENSION ||
-      type == Manifest::TYPE_LEGACY_PACKAGED_APP ||
-      (type == Manifest::TYPE_PLATFORM_APP &&
-       extension->location() == Manifest::COMPONENT)) {
-    content::ChildProcessSecurityPolicy::GetInstance()->GrantScheme(
-        process->GetID(), content::kChromeUIScheme);
-  }
 
   // Some extensions use file:// URLs.
   if (type == Manifest::TYPE_EXTENSION ||
@@ -95,7 +83,7 @@
     ExtensionPrefs* prefs = ExtensionPrefs::Get(browser_context_);
     if (prefs->AllowFileAccess(extension->id())) {
       content::ChildProcessSecurityPolicy::GetInstance()->GrantScheme(
-          process->GetID(), url::kFileScheme);
+          render_view_host->GetProcess()->GetID(), url::kFileScheme);
     }
   }