Only disallow top-level navigations in platform apps

content/renderer/render_view_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/renderer/render_view_impl.h"
 
 #include <algorithm>
 #include <cmath>
 #include <string>
 #include <vector>
@@ skipping 2344 matching lines @@
     WebString origin_str = frame->document().securityOrigin().toString();
     GURL frame_url(origin_str.utf8().data());
     // TODO(cevans): revisit whether this origin check is still necessary once
     // crbug.com/101395 is fixed.
     if (frame_url.GetOrigin() != url.GetOrigin()) {
       OpenURL(frame, url, referrer, default_policy);
       return WebKit::WebNavigationPolicyIgnore;
     }
   }
 
-  // If the browser is interested, then give it a chance to look at top level
-  // navigations.
+  // If the browser is interested, then give it a chance to look at the request.
   if (is_content_initiated) {
-    bool browser_handles_top_level_requests =
+    bool browser_handles_request =
         renderer_preferences_.browser_handles_top_level_requests &&
         IsNonLocalTopLevelNavigation(url, frame, type);
-    if (browser_handles_top_level_requests ||
-        renderer_preferences_.browser_handles_all_requests) {
+    if (!browser_handles_request) {
+      browser_handles_request = renderer_preferences_.
+              browser_handles_all_top_level_or_non_local_requests &&
+          IsNonLocalOrTopLevelNavigation(url, frame);
+    }
+
+    if (browser_handles_request) {
       // Reset these counters as the RenderView could be reused for the next
       // navigation.
       page_id_ = -1;
       last_page_id_sent_to_browser_ = -1;
       OpenURL(frame, url, referrer, default_policy);
       return WebKit::WebNavigationPolicyIgnore;  // Suppress the load here.
     }
   }
 
   // Detect when we're crossing a permission-based boundary (e.g. into or out of
@@ skipping 2821 matching lines @@
                                             &override_state))
     return override_state;
   return current_state;
 }
 
 WebKit::WebUserMediaClient* RenderViewImpl::userMediaClient() {
   EnsureMediaStreamImpl();
   return media_stream_impl_;
 }
 
+bool RenderViewImpl::IsNonLocalOrTopLevelNavigation(

[darin (slow to review), 2012/05/12 00:00:51]

it seems like you are using "NonLocal" to mean !same-origin.  maybe it would be
good to use the word "origin" in the function name?  it is a little bit longer,
but IsDifferentOriginOrTopLevelNavigation seems a bit clearer to me.

[Mihai Parparita -not on Chrome, 2012/05/15 20:51:46]

The problem is that data: URLs are allowed too, so it's not just about strict
origin checks.

How about IsRemoteOrTopLevelNavigation?

[darin (slow to review), 2012/05/15 23:58:44]

What about blob URLs or filesystem URLs?  I think I'm pretty confused about the
desired policy.  Blob/Filesystem URLs have an embedded origin, and yet they are
local resources.

[Mihai Parparita -not on Chrome, 2012/05/16 00:36:45]

Both of those should be allowed (added them to the test).

filesystem: URLs are handled by GURL::GetOrigin (it returns the inner URL's
origin, see
http://code.google.com/searchframe#OAMlx_jo-ck/src/googleurl/src/gurl.cc&exac...).
blob: ones aren't, I've added them to the special-case list along with data:
URLs.

+    const GURL& url, WebKit::WebFrame* frame) const {
+  if (frame->parent() == NULL)
+    return true;
+
+  // data: URLs don't involve remote content either, thus are considered local.
+  if (url.SchemeIs(chrome::kDataScheme))
+    return false;
+
+  return url.GetOrigin() != GURL(frame->top()->document().url()).GetOrigin();

[darin (slow to review), 2012/05/12 00:00:51]

why do you check the origin of frame->top() as opposed to just the origin of the
frame?  should you be comparing WebSecurityOrigin objects instead?

[Mihai Parparita -not on Chrome, 2012/05/15 20:51:46]

The frame hasn't navigated yet, so it's at about:blank, so its origin isn't
relevant. If I use WebSecurityOrigin (presumably you meant the canRequest
method), then I get some undesired side-effects:
- canRequest returns false for data: URLs
- canRequest returns true for origins that the app is whitelisted to load data
from (via its permissions). However, we still don't want to allow loading of
those remote URLs.

[darin (slow to review), 2012/05/15 23:58:44]

But what if the top-most frame was created dynamically via window.open(), and
then the opener just scribbled some HTML into it?  The URL of the top-most frame
would be about:blank in this case, yet its security origin would be that of the
opener.  What if that window contains an IFRAME that is in the same origin as
the top-most window and opener?  The URL-based checks fail to capture subtle
cases like this, right?

[Mihai Parparita -not on Chrome, 2012/05/16 00:36:45]

I've switched to using the document's security origin. It feels a bit weird to
be turning a WebSecurityOrigin into a GURL, but I guess that toString() docs say
it is a valid URL, and other places to it too:

http://code.google.com/p/chromium/source/search?q=GURL.*securityOrigin.*toStr...

BTW, I was following the pattern of IsNonLocalTopLevelNavigation (see below) for
comparing origins. I think that needs to be fixed too, but I think it's for
ChromeFrame usage, so I'm scared to touch it.

+}
+
 bool RenderViewImpl::IsNonLocalTopLevelNavigation(
-    const GURL& url, WebKit::WebFrame* frame, WebKit::WebNavigationType type) {
+    const GURL& url, WebKit::WebFrame* frame, WebKit::WebNavigationType type)
+    const {
   // Must be a top level frame.
   if (frame->parent() != NULL)
     return false;
 
   // Navigations initiated within Webkit are not sent out to the external host
   // in the following cases.
   // 1. The url scheme is not http/https
   // 2. The origin of the url and the opener is the same in which case the
   //    opener relationship is maintained.
   // 3. Reloads/form submits/back forward navigations
@@ skipping 74 matching lines @@
 bool RenderViewImpl::WebWidgetHandlesCompositorScheduling() const {
   return !!RenderThreadImpl::current()->compositor_thread();
 }
 
 void RenderViewImpl::OnJavaBridgeInit() {
   DCHECK(!java_bridge_dispatcher_.get());
 #if defined(ENABLE_JAVA_BRIDGE)
   java_bridge_dispatcher_.reset(new JavaBridgeDispatcher(this));
 #endif
 }