Prevent unauthorized commits of the Chrome Web Store URL.

content/browser/renderer_host/render_view_host_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/renderer_host/render_view_host_impl.h"
 
 #include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 1184 matching lines @@
       main_frame_id_ = validated_params.frame_id;
     } else {
       // TODO(nasko): We plan to remove the usage of frame_id in navigation
       // and move to routing ids. This is in place to ensure that a
       // renderer is not misbehaving and sending us incorrect data.
       DCHECK_EQ(main_frame_id_, validated_params.frame_id);
     }
   }
   RenderProcessHost* process = GetProcess();
 
-  // If the --site-per-process flag is passed, then the renderer process is
-  // not allowed to request web pages from other sites than the one it is
-  // dedicated to.
-  // Kill the renderer process if it violates this policy.
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kSitePerProcess) &&
-      static_cast<SiteInstanceImpl*>(GetSiteInstance())->HasSite() &&
-      validated_params.url != GURL(chrome::kAboutBlankURL)) {
-    if (!SiteInstance::IsSameWebSite(GetSiteInstance()->GetBrowserContext(),
-                                     GetSiteInstance()->GetSiteURL(),
-                                     validated_params.url) ||
-        static_cast<SiteInstanceImpl*>(GetSiteInstance())->
-            HasWrongProcessForURL(validated_params.url)) {
-      // TODO(nasko): Removed the actual kill process call until out-of-process
-      // iframes is ready to go.
-    }
+  // Attempts to commit certain off-limits URL should be caught more strictly
+  // than our FilterURL checks below.  If a renderer violates this policy, it
+  // should be killed.
+  if (!CanCommitURL(validated_params.url)) {
+    VLOG(1) << "Blocked URL " << validated_params.url.spec();
+    validated_params.url = GURL(chrome::kAboutBlankURL);
+    RecordAction(UserMetricsAction("CanCommitURL_BlockedAndKilled"));
+    base::KillProcess(
+        process->GetHandle(), content::RESULT_CODE_KILLED, false);

[nasko, 2013/05/10 20:37:34]

Why not call ReceivedBadMessage? It checks for single process mode and possibly
other things in the future.

[Charlie Reis, 2013/05/10 22:35:01]

Good idea.  Fixed.

(The one downside is that it has a NOTREACHED and kills the browser process in
debug builds, but that's arguably a good thing.  Catch it early if it's a bug,
and be aware if it's an exploit.  Might make it trickier to test, though.)

   }
 
   ChildProcessSecurityPolicyImpl* policy =
       ChildProcessSecurityPolicyImpl::GetInstance();
   // Without this check, an evil renderer can trick the browser into creating
   // a navigation entry for a banned URL.  If the user clicks the back button
   // followed by the forward button (or clicks reload, or round-trips through
   // session restore, etc), we'll think that the browser commanded the
   // 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
@@ skipping 468 matching lines @@
 #endif
 
 void RenderViewHostImpl::SendOrientationChangeEvent(int orientation) {
   Send(new ViewMsg_OrientationChangeEvent(GetRoutingID(), orientation));
 }
 
 void RenderViewHostImpl::ToggleSpeechInput() {
   Send(new InputTagSpeechMsg_ToggleSpeechInput(GetRoutingID()));
 }
 
+bool RenderViewHostImpl::CanCommitURL(const GURL& url) {
+  // TODO(creis): We should also check for WebUI pages here.  Also, when the
+  // out-of-process iframes implementation is ready, we should check for
+  // cross-site URLs that are not allowed to commit in this process.
+
+  // Give the client a chance to disallow URLs from committing.
+  return GetContentClient()->browser()->CanCommitURL(GetProcess(), url);
+}
+
 void RenderViewHostImpl::FilterURL(ChildProcessSecurityPolicyImpl* policy,
                                    const RenderProcessHost* process,
                                    bool empty_allowed,
                                    GURL* url) {
   if (empty_allowed && url->is_empty())
     return;
 
   // The browser process should never hear the swappedout:// URL from any
   // of the renderer's messages.  Check for this in debug builds, but don't
   // let it crash a release browser.
@@ skipping 360 matching lines @@
       webkit_glue::FilePathsFromHistoryState(state);
   for (std::vector<base::FilePath>::const_iterator file = file_paths.begin();
        file != file_paths.end(); ++file) {
     if (!policy->CanReadFile(GetProcess()->GetID(), *file))
       return false;
   }
   return true;
 }
 
 }  // namespace content