Disallow renderer-init BeginNavigation() to be same-document and verify.

Step 3 for bug 1125106. This is a subset of the mega-patch in
https://chromium-review.googlesource.com/c/chromium/src/+/2462248.

The BeginNavigation() path does not read the same-document field when
deciding its navigation type. However, a regression could conceivably
allow the renderer to specify same-document, bypass our checks to move
into a new RenderFrame/RenderProcess, and load a document in the wrong
process.

We drop BeginNavigation() messages from the renderer which have the
same-document flag set. Then we verify in RenderFrameHostManager when we
bypass from picking a RenderFrameHost, using the current one explicitly,
that the NavigationRequest did not come from BeginNavigation().

R=nasko@chromium.org

Bug: 1125106
Change-Id: I58d50f524bf948c5ca4f76c237c6fe32e7fcf4ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2558776
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831174}

content/browser/renderer_host/ipc_utils.cc

@@ -12,6 +12,7 @@
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/common/frame.mojom.h"
 #include "content/common/frame_messages.h"
+#include "content/common/navigation_params_utils.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_process_host.h"
@@ -188,6 +189,12 @@ bool VerifyBeginNavigationCommonParams(
     return false;
   }
 
+  // Asynchronous (browser-controlled, but) renderer-initiated navigations can
+  // not be same-document. Allowing this incorrectly could have us try to
+  // navigate an existing document to a different site.
+  if (NavigationTypeUtils::IsSameDocument(common_params->navigation_type))
+    return false;
+
   // Verification succeeded.
   return true;
 }

content/browser/renderer_host/render_frame_host_manager.cc

@@ -688,13 +688,30 @@ bool RenderFrameHostManager::HasPendingCommitForCrossDocumentNavigation()
 
 void RenderFrameHostManager::DidCreateNavigationRequest(
     NavigationRequest* request) {
-  if (request->IsServedFromBackForwardCache()) {
+  const bool force_use_current_render_frame_host =
+      // Since the frame from the back-forward cache is being committed to the
+      // SiteInstance we already have, it is treated as current.
+      request->IsServedFromBackForwardCache();
+
+  if (force_use_current_render_frame_host) {
+    // This method should generally be calling GetFrameHostForNavigation() in
+    // order to choose the correct RenderFrameHost, and choose a speculative
+    // RenderFrameHost when the navigation can not be performed in the current
+    // frame. Getting this wrong has security consequences as it could allow a
+    // document from a different security context to be loaded in the current
+    // frame and gain access to things in-process that it should not.
+    // However, there are some situations where we know that we want to perform
+    // the navigation in the current frame. In that case we must be sure that
+    // the renderer is not *controlling* the navigation. The BeginNavigation()
+    // path allows the renderer to specify all the parameters of the
+    // NavigationRequest, so we should never allow it to specify that the
+    // navigation be performed in the current RenderFrameHost.
+    CHECK(!request->from_begin_navigation());
+
     // Cleanup existing pending RenderFrameHost. This corresponds to what is
-    // done inside GetFrameHostForNavigation(request), but this isn't called
-    // with the back-forward cache.
+    // done inside GetFrameHostForNavigation(request), but we avoid calling that
+    // method for navigations which will be forced into the current document.
     CleanUpNavigation();
-    // Since the frame from the back-forward cache is being committed to the
-    // SiteInstance we already have, it is treated as current.
     request->set_associated_site_instance_type(
         NavigationRequest::AssociatedSiteInstanceType::CURRENT);
   } else {