Prevent the browser process from creating duplicate RenderViewHosts.

content/browser/security_exploit_browsertest.cc

Index: content/browser/security_exploit_browsertest.cc
diff --git a/content/browser/security_exploit_browsertest.cc b/content/browser/security_exploit_browsertest.cc
index 2eb7b51885b48168df9a248d249eee2bc44195dd..4bb84b1473dc0b8076c15ea92788cd8132e547c1 100644
--- a/content/browser/security_exploit_browsertest.cc
+++ b/content/browser/security_exploit_browsertest.cc
@@ -3,11 +3,19 @@
 // found in the LICENSE file.
 
 #include "base/command_line.h"
+#include "base/containers/hash_tables.h"
+#include "content/browser/dom_storage/dom_storage_context_wrapper.h"
+#include "content/browser/dom_storage/session_storage_namespace_impl.h"
+#include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
+#include "content/common/view_messages.h"
+#include "content/public/browser/browser_context.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
+#include "content/public/browser/storage_partition.h"
 #include "content/public/common/content_switches.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/content_browser_test.h"
@@ -15,6 +23,61 @@
 
 namespace content {
 
+namespace {
+
+// This is a helper function for the tests which attempt to create a
+// duplicate RenderViewHost or RenderWidgetHost. It tries to create two objects
+// with the same process and routing ids, which causes a collision.
+// It creates a couple of windows in process 1, which causes a few routing ids
+// to be allocated. Then a cross-process navigation is initiated, which causes a
+// new process 2 to be created and have a pending RenderViewHost for it. The
+// routing id of the RenderViewHost which is target for a duplicate is set
+// into |target_routing_id| and the pending RenderViewHost which is used for
+// the attempt is the return value.
+RenderViewHostImpl* PrepareToDuplicateHosts(Shell* shell,
+                                            int* target_routing_id) {
+  GURL foo("http://foo.com/files/simple_page.html");
+
+  // Start off with initial navigation, so we get the first process allocated.
+  NavigateToURL(shell, foo);
+
+  // Open another window, so we generate some more routing ids.
+  ShellAddedObserver shell2_observer;
+  EXPECT_TRUE(ExecuteScript(
+      shell->web_contents(), "window.open(document.URL + '#2');"));
+  Shell* shell2 = shell2_observer.GetShell();
+
+  // The new window must be in the same process, but have a new routing id.
+  EXPECT_EQ(shell->web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
+            shell2->web_contents()->GetRenderViewHost()->GetProcess()->GetID());
+  *target_routing_id =
+      shell2->web_contents()->GetRenderViewHost()->GetRoutingID();
+  EXPECT_NE(*target_routing_id,
+            shell->web_contents()->GetRenderViewHost()->GetRoutingID());
+
+  // Now, simulate a link click coming from the renderer.
+  GURL extension_url("https://bar.com/files/simple_page.html");
+  WebContentsImpl* wc = static_cast<WebContentsImpl*>(shell->web_contents());
+  wc->RequestOpenURL(
+      shell->web_contents()->GetRenderViewHost(), extension_url,
+      Referrer(), CURRENT_TAB, wc->GetFrameTree()->root()->frame_id(),
+      false, true);
+
+  // Since the navigation above requires a cross-process swap, there will be a
+  // pending RenderViewHost. Ensure it exists and is in a different process
+  // than the initial page.
+  RenderViewHostImpl* pending_rvh =
+      wc->GetRenderManagerForTesting()->pending_render_view_host();
+  EXPECT_TRUE(pending_rvh != NULL);
+  EXPECT_NE(shell->web_contents()->GetRenderViewHost()->GetProcess()->GetID(),
+            pending_rvh->GetProcess()->GetID());
+
+  return pending_rvh;
+}
+
+}  // namespace
+
+
 // The goal of these tests will be to "simulate" exploited renderer processes,
 // which can send arbitrary IPC messages and confuse browser process internal
 // state, leading to security bugs. We are trying to verify that the browser
@@ -52,4 +115,35 @@ IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, SetWebUIProperty) {
   terminated.Wait();
 }
 
+// This is a test for crbug.com/312016 attempting to create duplicate
+// RenderViewHosts. SetupForDuplicateHosts sets up this test case and leaves
+// it in a state with pending RenderViewHost. Before the commit of the new
+// pending RenderViewHost, this test case creates a new window through the new
+// process.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest,
+                       AttemptDuplicateRenderViewHost) {
+  int duplicate_routing_id = MSG_ROUTING_NONE;
+  RenderViewHostImpl* pending_rvh =
+      PrepareToDuplicateHosts(shell(), &duplicate_routing_id);
+  EXPECT_NE(MSG_ROUTING_NONE, duplicate_routing_id);
+
+  // Since this test executes on the UI thread and hopping threads might cause
+  // different timing in the test, let's simulate a CreateNewWindow call coming
+  // from the IO thread.
+  ViewHostMsg_CreateWindow_Params params;
+  DOMStorageContextWrapper* dom_storage_context =
+      static_cast<DOMStorageContextWrapper*>(
+          BrowserContext::GetStoragePartition(
+              shell()->web_contents()->GetBrowserContext(),
+              pending_rvh->GetSiteInstance())->GetDOMStorageContext());
+  SessionStorageNamespaceImpl* session_storage =
+      new SessionStorageNamespaceImpl(dom_storage_context);
+  // Cause a deliberate collision in routing ids.
+  int main_frame_routing_id = duplicate_routing_id + 1;
+  pending_rvh->CreateNewWindow(
+      duplicate_routing_id, main_frame_routing_id, params, session_storage);
+
+  // If the above operation doesn't cause a crash, the test has succeeded!
 }
+
+}  // namespace content