Validate params.origin in the browser process at commit time.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 3b619f154c6fca9d60b09d3a99f30145ba36c663..ebbd3cceba9ccb1e09e4094b54715e97461967ed 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -7,6 +7,7 @@
 #include <utility>
 
 #include "base/bind.h"
+#include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
@@ -71,6 +72,7 @@
 #include "content/public/browser/user_metrics.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_constants.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/isolated_world_ids.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
@@ -992,6 +994,15 @@ void RenderFrameHostImpl::OnDidCommitProvisionalLoad(const IPC::Message& msg) {
     // Kills the process.
     bad_message::ReceivedBadMessage(process,
                                     bad_message::RFH_CAN_COMMIT_URL_BLOCKED);
+    return;
+  }
+
+  // Verify that the origin passed from the renderer process is valid and can
+  // be allowed to commit in this RenderFrameHost.
+  if (!CanCommitOrigin(validated_params.url, validated_params.origin)) {
+    bad_message::ReceivedBadMessage(GetProcess(),
+                                    bad_message::RFH_INVALID_ORIGIN_ON_COMMIT);
+    return;
   }
 
   // Without this check, an evil renderer can trick the browser into creating
@@ -1930,6 +1941,46 @@ bool RenderFrameHostImpl::CanCommitURL(const GURL& url) {
   return GetContentClient()->browser()->CanCommitURL(GetProcess(), url);
 }
 
+bool RenderFrameHostImpl::CanCommitOrigin(
+    const GURL& url,
+    const url::Origin& origin) {
+  // If the --disable-web-security flag is specified, all bets are off and the
+  // renderer process can send any origin it wishes.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+      switches::kDisableWebSecurity)) {
+    return true;
+  }
+
+  // file: URLs can be allowed to access any other origin, based on settings.
+  if (origin.scheme() == url::kFileScheme) {
+    WebPreferences prefs = render_view_host_->GetWebkitPreferences();
+    if (prefs.allow_universal_access_from_file_urls)
+      return true;
+  }
+
+  // It is safe to commit into a unique origin, regardless of the URL, as it is
+  // restricted from accessing other origins.
+  if (origin.unique())
+    return true;
+
+  // Non-unique origin must be a valid URL, which allows us to safely do a

[Charlie Reis, 2016/03/10 22:28:02]

nit: A non-unique origin

[nasko, 2016/03/11 00:19:37]

Done.

+  // conversion to GURL.
+  GURL origin_url(origin.Serialize());
+
+  // Standard URLs can be safely converted to an origin, but not vice-versa.
+  // If the actual URL is not standard, check the origin URL whether it is
+  // allowed to commit.
+  // Note: This handles cases such as 'about:blank', data and blob URLs.
+  if (!url.IsStandard())
+    return CanCommitURL(origin_url);
+
+  // Standard URLs must match the reported origin.
+  if (!origin.IsSameOriginWith(url::Origin(url)))
+    return false;
+
+  return CanCommitURL(origin_url);

[Charlie Reis, 2016/03/10 22:28:02]

Feels weird to call CanCommitURL twice.  Maybe we can rephrase lines 1970-1981
as the following?

  // Standard URLs must match the reported origin.
  if (url.IsStandard() &&
      !origin.IsSameOriginWith(url::Origin(url))) {
    return false;
  }

  // Verify that the origin is allowed to commit in this process.
  // Note: This also handles non-standard cases for |url|, such as
  // about:blank, data, and blob URLs.
  return CanCommitURL(origin_url);

[nasko, 2016/03/11 00:19:37]

Done.

+}
+
 void RenderFrameHostImpl::Navigate(
     const CommonNavigationParams& common_params,
     const StartNavigationParams& start_params,