Prevent webview tags from navigating outside web-safe schemes.

This CL removes protocol handlers and avoids granting capabilities or
bindings to webview processes, which prevents navigations to WebUI,
extension, and file URLs.  Web and data URLs are still permitted.

BUG=139397
TEST=Try to visit chrome://settings or other privileged URLs in a <webview>.


Review URL: https://chromiumcodereview.appspot.com/11313018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@164788 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 97 insertions, 68 deletions

diff --git a/content/browser/browser_plugin/browser_plugin_embedder.cc b/content/browser/browser_plugin/browser_plugin_embedder.cc
index 4f23410..1c83286 100644
--- a/content/browser/browser_plugin/browser_plugin_embedder.cc
+++ b/content/browser/browser_plugin/browser_plugin_embedder.cc
@@ -142,7 +142,10 @@ void BrowserPluginEmbedder::NavigateGuest(
   // should never be sent to BrowserPluginEmbedder (browser process).
   DCHECK(!src.empty());
   if (!src.empty()) {
-    // TODO(creis): Check the validity of the URL: http://crbug.com/139397.
+    // Because guests do not swap processes on navigation, only navigations to
+    // normal web URLs are supported.  No protocol handlers are installed for
+    // other schemes (e.g., WebUI or extensions), and no permissions or bindings
+    // can be granted to the guest process.
     guest_web_contents->GetController().LoadURL(url,
                                                 Referrer(),
                                                 PAGE_TRANSITION_AUTO_TOPLEVEL,
diff --git a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
index ecd62b0..e5d9620 100644
--- a/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
+++ b/content/browser/browser_plugin/browser_plugin_host_browsertest.cc
@@ -746,19 +746,47 @@ IN_PROC_BROWSER_TEST_F(BrowserPluginHostTest, LoadAbort) {
   const char kEmbedderURL[] = "files/browser_plugin_embedder.html";
   StartBrowserPluginTest(kEmbedderURL, "about:blank", true, "");
 
-  const string16 expected_title = ASCIIToUTF16("ERR_EMPTY_RESPONSE");
-  content::TitleWatcher title_watcher(test_embedder()->web_contents(),
-                                      expected_title);
+  {
+    // Navigate the guest to "close-socket".
+    const string16 expected_title = ASCIIToUTF16("ERR_EMPTY_RESPONSE");
+    content::TitleWatcher title_watcher(test_embedder()->web_contents(),
+                                        expected_title);
+    RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
+        test_embedder()->web_contents()->GetRenderViewHost());
+    GURL test_url = test_server()->GetURL("close-socket");
+    ExecuteSyncJSFunction(rvh, ASCIIToUTF16(
+        StringPrintf("SetSrc('%s');", test_url.spec().c_str())));
+    string16 actual_title = title_watcher.WaitAndGetTitle();
+    EXPECT_EQ(expected_title, actual_title);
+  }
 
-  // Renavigate the guest to "close-socket".
-  RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
-      test_embedder()->web_contents()->GetRenderViewHost());
-  GURL test_url = test_server()->GetURL("close-socket");
-  ExecuteSyncJSFunction(rvh, ASCIIToUTF16(
-      StringPrintf("SetSrc('%s');", test_url.spec().c_str())));
+  {
+    // Navigate the guest to an illegal chrome:// URL.
+    const string16 expected_title = ASCIIToUTF16("ERR_FAILED");
+    content::TitleWatcher title_watcher(test_embedder()->web_contents(),
+                                        expected_title);
+    RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
+        test_embedder()->web_contents()->GetRenderViewHost());
+    GURL test_url("chrome://newtab");
+    ExecuteSyncJSFunction(rvh, ASCIIToUTF16(
+        StringPrintf("SetSrc('%s');", test_url.spec().c_str())));
+    string16 actual_title = title_watcher.WaitAndGetTitle();
+    EXPECT_EQ(expected_title, actual_title);
+  }
 
-  string16 actual_title = title_watcher.WaitAndGetTitle();
-  EXPECT_EQ(expected_title, actual_title);
+  {
+    // Navigate the guest to an illegal file:// URL.
+    const string16 expected_title = ASCIIToUTF16("ERR_ABORTED");
+    content::TitleWatcher title_watcher(test_embedder()->web_contents(),
+                                        expected_title);
+    RenderViewHostImpl* rvh = static_cast<RenderViewHostImpl*>(
+        test_embedder()->web_contents()->GetRenderViewHost());
+    GURL test_url("file://foo");
+    ExecuteSyncJSFunction(rvh, ASCIIToUTF16(
+        StringPrintf("SetSrc('%s');", test_url.spec().c_str())));
+    string16 actual_title = title_watcher.WaitAndGetTitle();
+    EXPECT_EQ(expected_title, actual_title);
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(BrowserPluginHostTest, LoadRedirect) {
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index e6cdcc9..75de051 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -131,11 +131,11 @@ RenderViewHost* RenderViewHost::From(RenderWidgetHost* rwh) {
 }
 
 // static
-void RenderViewHost::FilterURL(int renderer_id,
+void RenderViewHost::FilterURL(const RenderProcessHost* process,
                                bool empty_allowed,
                                GURL* url) {
   RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl::GetInstance(),
-                                renderer_id, empty_allowed, url);
+                                process, empty_allowed, url);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -268,6 +268,8 @@ bool RenderViewHostImpl::CreateRenderView(
 
   // If it's enabled, tell the renderer to set up the Javascript bindings for
   // sending messages back to the browser.
+  if (GetProcess()->IsGuest())
+    DCHECK_EQ(0, enabled_bindings_);
   Send(new ViewMsg_AllowBindings(GetRoutingID(), enabled_bindings_));
   // Let our delegate know that we created a RenderView.
   delegate_->RenderViewCreated(this);
@@ -289,14 +291,18 @@ void RenderViewHostImpl::SyncRendererPrefs() {
 }
 
 void RenderViewHostImpl::Navigate(const ViewMsg_Navigate_Params& params) {
-  ChildProcessSecurityPolicyImpl::GetInstance()->GrantRequestURL(
-      GetProcess()->GetID(), params.url);
-  if (params.url.SchemeIs(chrome::kDataScheme) &&
-      params.base_url_for_data_url.SchemeIs(chrome::kFileScheme)) {
-    // If 'data:' is used, and we have a 'file:' base url, grant access to
-    // local files.
+  // Browser plugin guests are not allowed to navigate outside web-safe schemes,
+  // so do not grant them the ability to request additional URLs.
+  if (!GetProcess()->IsGuest()) {
     ChildProcessSecurityPolicyImpl::GetInstance()->GrantRequestURL(
-        GetProcess()->GetID(), params.base_url_for_data_url);
+        GetProcess()->GetID(), params.url);
+    if (params.url.SchemeIs(chrome::kDataScheme) &&
+        params.base_url_for_data_url.SchemeIs(chrome::kFileScheme)) {
+      // If 'data:' is used, and we have a 'file:' base url, grant access to
+      // local files.
+      ChildProcessSecurityPolicyImpl::GetInstance()->GrantRequestURL(
+          GetProcess()->GetID(), params.base_url_for_data_url);
+    }
   }
 
   ViewMsg_Navigate* nav_message = new ViewMsg_Navigate(GetRoutingID(), params);
@@ -591,7 +597,7 @@ void RenderViewHostImpl::DragTargetDragEnter(
   // The URL could have been cobbled together from any highlighted text string,
   // and can't be interpreted as a capability.
   WebDropData filtered_data(drop_data);
-  FilterURL(policy, renderer_id, true, &filtered_data.url);
+  FilterURL(policy, GetProcess(), true, &filtered_data.url);
 
   // The filenames vector, on the other hand, does represent a capability to
   // access the given files.
@@ -817,6 +823,12 @@ void RenderViewHostImpl::AllowBindings(int bindings_flags) {
       return;
   }
 
+  // Never grant any bindings to browser plugin guests.
+  if (GetProcess()->IsGuest()) {
+    NOTREACHED() << "Never grant bindings to a guest process.";
+    return;
+  }
+
   if (bindings_flags & BINDINGS_POLICY_WEB_UI) {
     ChildProcessSecurityPolicyImpl::GetInstance()->GrantWebUIBindings(
         GetProcess()->GetID());
@@ -1221,7 +1233,7 @@ void RenderViewHostImpl::OnMsgNavigate(const IPC::Message& msg) {
   if (is_waiting_for_unload_ack_)
     return;
 
-  const int renderer_id = GetProcess()->GetID();
+  RenderProcessHost* process = GetProcess();
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
   // Without this check, an evil renderer can trick the browser into creating
@@ -1231,15 +1243,15 @@ void RenderViewHostImpl::OnMsgNavigate(const IPC::Message& msg) {
   // renderer to load the URL and grant the renderer the privileges to request
   // the URL.  To prevent this attack, we block the renderer from inserting
   // banned URLs into the navigation controller in the first place.
-  FilterURL(policy, renderer_id, false, &validated_params.url);
-  FilterURL(policy, renderer_id, true, &validated_params.referrer.url);
+  FilterURL(policy, process, false, &validated_params.url);
+  FilterURL(policy, process, true, &validated_params.referrer.url);
   for (std::vector<GURL>::iterator it(validated_params.redirects.begin());
       it != validated_params.redirects.end(); ++it) {
-    FilterURL(policy, renderer_id, false, &(*it));
+    FilterURL(policy, process, false, &(*it));
   }
-  FilterURL(policy, renderer_id, true, &validated_params.searchable_form_url);
-  FilterURL(policy, renderer_id, true, &validated_params.password_form.origin);
-  FilterURL(policy, renderer_id, true, &validated_params.password_form.action);
+  FilterURL(policy, process, true, &validated_params.searchable_form_url);
+  FilterURL(policy, process, true, &validated_params.password_form.origin);
+  FilterURL(policy, process, true, &validated_params.password_form.action);
 
   delegate_->DidNavigate(this, validated_params);
 
@@ -1323,16 +1335,16 @@ void RenderViewHostImpl::OnMsgContextMenu(const ContextMenuParams& params) {
   // Validate the URLs in |params|.  If the renderer can't request the URLs
   // directly, don't show them in the context menu.
   ContextMenuParams validated_params(params);
-  int renderer_id = GetProcess()->GetID();
+  RenderProcessHost* process = GetProcess();
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   // We don't validate |unfiltered_link_url| so that this field can be used
   // when users want to copy the original link URL.
-  FilterURL(policy, renderer_id, true, &validated_params.link_url);
-  FilterURL(policy, renderer_id, true, &validated_params.src_url);
-  FilterURL(policy, renderer_id, false, &validated_params.page_url);
-  FilterURL(policy, renderer_id, true, &validated_params.frame_url);
+  FilterURL(policy, process, true, &validated_params.link_url);
+  FilterURL(policy, process, true, &validated_params.src_url);
+  FilterURL(policy, process, false, &validated_params.page_url);
+  FilterURL(policy, process, true, &validated_params.frame_url);
 
   ContextMenuSourceType type = CONTEXT_MENU_SOURCE_MOUSE;
   if (!in_process_event_types_.empty()) {
@@ -1357,7 +1369,7 @@ void RenderViewHostImpl::OnMsgOpenURL(const GURL& url,
                                       int64 source_frame_id) {
   GURL validated_url(url);
   FilterURL(ChildProcessSecurityPolicyImpl::GetInstance(),
-            GetProcess()->GetID(), false, &validated_url);
+            GetProcess(), false, &validated_url);
 
   delegate_->RequestOpenURL(
       this, validated_url, referrer, disposition, source_frame_id);
@@ -1452,13 +1464,14 @@ void RenderViewHostImpl::OnMsgStartDragging(
     return;
 
   WebDropData filtered_data(drop_data);
+  RenderProcessHost* process = GetProcess();
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   // Allow drag of Javascript URLs to enable bookmarklet drag to bookmark bar.
   if (!filtered_data.url.SchemeIs(chrome::kJavaScriptScheme))
-    FilterURL(policy, GetProcess()->GetID(), true, &filtered_data.url);
-  FilterURL(policy, GetProcess()->GetID(), false, &filtered_data.html_base_url);
+    FilterURL(policy, process, true, &filtered_data.url);
+  FilterURL(policy, process, false, &filtered_data.html_base_url);
   // Filter out any paths that the renderer didn't have access to. This prevents
   // the following attack on a malicious renderer:
   // 1. StartDragging IPC sent with renderer-specified filesystem paths that it
@@ -1699,7 +1712,7 @@ void RenderViewHostImpl::ToggleSpeechInput() {
 }
 
 void RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl* policy,
-                                   int renderer_id,
+                                   const RenderProcessHost* process,
                                    bool empty_allowed,
                                    GURL* url) {
   if (empty_allowed && url->is_empty())
@@ -1720,7 +1733,12 @@ void RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl* policy,
     *url = GURL(chrome::kAboutBlankURL);
   }
 
-  if (!policy->CanRequestURL(renderer_id, *url)) {
+  // Do not allow browser plugin guests to navigate to non-web URLs, since they
+  // cannot swap processes or grant bindings.
+  bool non_web_url_in_guest = process->IsGuest() &&
+      !(url->is_valid() && policy->IsWebSafeScheme(url->scheme()));
+
+  if (non_web_url_in_guest || !policy->CanRequestURL(process->GetID(), *url)) {
     // If this renderer is not permitted to request this URL, we invalidate the
     // URL.  This prevents us from storing the blocked URL and becoming confused
     // later.
diff --git a/content/browser/renderer_host/render_view_host_impl.h b/content/browser/renderer_host/render_view_host_impl.h
index c62a70ba..4e2f615 100644
--- a/content/browser/renderer_host/render_view_host_impl.h
+++ b/content/browser/renderer_host/render_view_host_impl.h
@@ -442,7 +442,7 @@ class CONTENT_EXPORT RenderViewHostImpl
   // about:blank.
   // empty_allowed must be set to false for navigations for security reasons.
   static void FilterURL(ChildProcessSecurityPolicyImpl* policy,
-                        int renderer_id,
+                        const RenderProcessHost* process,
                         bool empty_allowed,
                         GURL* url);
 
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 38713e0..c86b627 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -1961,14 +1961,8 @@ void WebContentsImpl::DidStartProvisionalLoadForFrame(
   GURL validated_opener_url(opener_url);
   RenderProcessHost* render_process_host =
       render_view_host->GetProcess();
-  RenderViewHost::FilterURL(
-      render_process_host->GetID(),
-      false,
-      &validated_url);
-  RenderViewHost::FilterURL(
-      render_process_host->GetID(),
-      true,
-      &validated_opener_url);
+  RenderViewHost::FilterURL(render_process_host, false, &validated_url);
+  RenderViewHost::FilterURL(render_process_host, true, &validated_opener_url);
 
   // Notify observers about the start of the provisional load.
   FOR_EACH_OBSERVER(WebContentsObserver, observers_,
@@ -1999,21 +1993,9 @@ void WebContentsImpl::DidRedirectProvisionalLoad(
   GURL validated_opener_url(opener_url);
   RenderProcessHost* render_process_host =
       render_view_host->GetProcess();
-  RenderViewHostImpl::FilterURL(
-      ChildProcessSecurityPolicyImpl::GetInstance(),
-      render_process_host->GetID(),
-      false,
-      &validated_source_url);
-  RenderViewHostImpl::FilterURL(
-      ChildProcessSecurityPolicyImpl::GetInstance(),
-      render_process_host->GetID(),
-      false,
-      &validated_target_url);
-  RenderViewHostImpl::FilterURL(
-      ChildProcessSecurityPolicyImpl::GetInstance(),
-      render_process_host->GetID(),
-      true,
-      &validated_opener_url);
+  RenderViewHost::FilterURL(render_process_host, false, &validated_source_url);
+  RenderViewHost::FilterURL(render_process_host, false, &validated_target_url);
+  RenderViewHost::FilterURL(render_process_host, true, &validated_opener_url);
   NavigationEntry* entry;
   if (page_id == -1) {
     entry = controller_.GetPendingEntry();
@@ -2044,10 +2026,7 @@ void WebContentsImpl::DidFailProvisionalLoadWithError(
   GURL validated_url(params.url);
   RenderProcessHost* render_process_host =
       render_view_host->GetProcess();
-  RenderViewHost::FilterURL(
-      render_process_host->GetID(),
-      false,
-      &validated_url);
+  RenderViewHost::FilterURL(render_process_host, false, &validated_url);
 
   if (net::ERR_ABORTED == params.error_code) {
     // EVIL HACK ALERT! Ignore failed loads when we're showing interstitials.
diff --git a/content/public/browser/render_view_host.h b/content/public/browser/render_view_host.h
index db2cff7..6597376 100644
--- a/content/public/browser/render_view_host.h
+++ b/content/public/browser/render_view_host.h
@@ -37,6 +37,7 @@ struct WebPluginAction;
 namespace content {
 
 class ChildProcessSecurityPolicy;
+class RenderProcessHost;
 class RenderViewHostDelegate;
 class SessionStorageNamespace;
 class SiteInstance;
@@ -66,7 +67,7 @@ class CONTENT_EXPORT RenderViewHost : virtual public RenderWidgetHost {
   // Checks that the given renderer can request |url|, if not it sets it to
   // about:blank.
   // |empty_allowed| must be set to false for navigations for security reasons.
-  static void FilterURL(int renderer_id,
+  static void FilterURL(const RenderProcessHost* process,
                         bool empty_allowed,
                         GURL* url);