RDH: Block a compromised renderer from reusing request ids

content/browser/security_exploit_browsertest.cc

 // Copyright (c) 2013 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 <stdint.h>
 
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "content/browser/dom_storage/dom_storage_context_wrapper.h"
 #include "content/browser/dom_storage/session_storage_namespace_impl.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_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/frame_messages.h"
 #include "content/common/resource_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/interstitial_page.h"
 #include "content/public/browser/interstitial_page_delegate.h"
+#include "content/public/browser/resource_dispatcher_host.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/common/appcache_info.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/file_chooser_params.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #include "content/test/test_content_browser_client.h"
 #include "ipc/ipc_security_test_util.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/url_request/url_request_slow_download_job.h"
 
 using IPC::IpcSecurityTestUtil;
 
 namespace content {
 
 namespace {
 
+const int kRequestIdNotPreviouslyUsed = 10000;

[mmenke, 2016/01/28 21:12:33]

nit:  Should have a comment about this.

[gzobqq, 2016/01/30 07:18:57]

Done.

+
 // 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,
@@ skipping 34 matching lines @@
   else
     next_rfh = wc->GetRenderManagerForTesting()->pending_frame_host();
 
   EXPECT_TRUE(next_rfh);
   EXPECT_NE(shell->web_contents()->GetRenderProcessHost()->GetID(),
             next_rfh->GetProcess()->GetID());
 
   return next_rfh->render_view_host();
 }
 
-ResourceHostMsg_Request CreateXHRRequestWithOrigin(const char* origin) {
+ResourceHostMsg_Request CreateXHRRequest(const char* url) {
   ResourceHostMsg_Request request;
   request.method = "GET";
-  request.url = GURL("http://bar.com/simple_page.html");
-  request.first_party_for_cookies = GURL(origin);
+  request.url = GURL(url);
   request.referrer_policy = blink::WebReferrerPolicyDefault;
-  request.headers = base::StringPrintf("Origin: %s\r\n", origin);
   request.load_flags = 0;
   request.origin_pid = 0;
   request.resource_type = RESOURCE_TYPE_XHR;
   request.request_context = 0;
   request.appcache_host_id = kAppCacheNoHostId;
   request.download_to_file = false;
   request.should_reset_appcache = false;
   request.is_main_frame = true;
   request.parent_is_main_frame = false;
   request.parent_render_frame_id = -1;
   request.transition_type = ui::PAGE_TRANSITION_LINK;
   request.allow_download = true;
   return request;
 }
 
+ResourceHostMsg_Request CreateXHRRequestWithOrigin(const char* origin) {
+  ResourceHostMsg_Request request = CreateXHRRequest("http://bar.com/simple_page.html");

[mmenke, 2016/01/28 21:12:33]

nit:  Split onto two lines, so no line is over 80 characters.

[Charlie Reis, 2016/01/28 22:05:16]

Tip: Running "git cl format" should take care of this automatically.  :)

[gzobqq, 2016/01/30 07:18:57]

Done.

[gzobqq, 2016/01/30 07:18:57]

That helps, thanks.

+  request.first_party_for_cookies = GURL(origin);
+  request.headers = base::StringPrintf("Origin: %s\r\n", origin);
+  return request;
+}
+
+void TryCreateDuplicateRequestIds(Shell *shell, bool block_loaders) {
+  NavigateToURL(shell, GURL("http://foo.com/simple_page.html"));
+  RenderFrameHost* web_rfh = shell->web_contents()->GetMainFrame();

[Charlie Reis, 2016/01/28 22:05:16]

nit: s/web_rfh/rfh/

(The web_ prefix was useful in the InvalidOriginHeaders test because the logic
was specific to RFHs in web processes, as opposed to chrome:// or other
privileged processes.  This test applies to all renderer processes.)

Same below with web_process_killed.

[gzobqq, 2016/01/30 07:18:57]

Done.

+
+  if (block_loaders) {
+    // Test the case where loaders are placed into blocked_loaders_map_.
+    int child_id = web_rfh->GetProcess()->GetID();
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&ResourceDispatcherHost::BlockRequestsForRoute,
+                   base::Unretained(ResourceDispatcherHost::Get()),
+                   child_id, web_rfh->GetRoutingID()));
+  }
+
+  // URLRequestSlowDownloadJob waits for another request to kFinishDownloadUrl
+  // to finish all pending requests. It is never sent, so the following URL
+  // blocks indefinitely, which is good because the request stays alive and the
+  // test can try to reuse the request id without a race.
+  const char* blocking_url = net::URLRequestSlowDownloadJob::kUnknownSizeUrl;
+  ResourceHostMsg_Request request(CreateXHRRequest(blocking_url));
+
+  RenderProcessHostWatcher web_process_killed(
+      web_rfh->GetProcess(),
+      RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  // Use the same request id twice.

[Charlie Reis, 2016/01/28 22:05:16]

nit: Move comment above the RenderProcessHostWatcher declaration, since it's a
good description of the whole block.

[gzobqq, 2016/01/30 07:18:58]

Done.

+  IPC::IpcSecurityTestUtil::PwnMessageReceived(
+      web_rfh->GetProcess()->GetChannel(),
+      ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                      kRequestIdNotPreviouslyUsed, request));
+  IPC::IpcSecurityTestUtil::PwnMessageReceived(
+      web_rfh->GetProcess()->GetChannel(),
+      ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                      kRequestIdNotPreviouslyUsed, request));
+  web_process_killed.Wait();
+}
+
 }  // 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
 // doesn't perform any dangerous operations in such cases.
 class SecurityExploitBrowserTest : public ContentBrowserTest {
  public:
   SecurityExploitBrowserTest() {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
     ASSERT_TRUE(embedded_test_server()->Start());
 
     // Add a host resolver rule to map all outgoing requests to the test server.
     // This allows us to use "real" hostnames in URLs, which we can use to
     // create arbitrary SiteInstances.
     command_line->AppendSwitchASCII(
         switches::kHostResolverRules,
         "MAP * " +
             net::HostPortPair::FromURL(embedded_test_server()->base_url())
                 .ToString() +
             ",EXCLUDE localhost");
   }
 
+  void SetUpOnMainThread() override {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        base::Bind(&net::URLRequestSlowDownloadJob::AddUrlHandler));
+  }
+
  protected:
   // Tests that a given file path sent in a ViewHostMsg_RunFileChooser will
   // cause renderer to be killed.
   void TestFileChooserWithPath(const base::FilePath& path);
 };
 
 void SecurityExploitBrowserTest::TestFileChooserWithPath(
     const base::FilePath& path) {
   GURL foo("http://foo.com/simple_page.html");
   NavigateToURL(shell(), foo);
@@ skipping 219 matching lines @@
   NavigateToURL(shell(), web_url);
   RenderFrameHost* web_rfh = shell()->web_contents()->GetMainFrame();
 
   // Web processes cannot make XHRs with chrome:// Origin headers.
   {
     RenderProcessHostWatcher web_process_killed(
         web_rfh->GetProcess(),
         RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
     IPC::IpcSecurityTestUtil::PwnMessageReceived(
         web_rfh->GetProcess()->GetChannel(),
-        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(), 1,
+        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                        kRequestIdNotPreviouslyUsed,
                                         chrome_origin_msg));
     web_process_killed.Wait();
   }
 
   // Web processes cannot make XHRs with URLs that the content embedder expects
   // to have process isolation.  Ideally this would test chrome-extension://
   // URLs for Chrome Apps, but those can't be tested inside content/ and the
   // ResourceHostMsg_Request IPC can't be created in a test outside content/.
   NavigateToURL(shell(), web_url);
   {
     // Set up a ContentBrowserClient that simulates an app URL in a non-app
     // process.
     IsolatedAppContentBrowserClient app_client;
     ContentBrowserClient* old_client = SetBrowserClientForTesting(&app_client);
     RenderProcessHostWatcher web_process_killed(
         web_rfh->GetProcess(),
         RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
     IPC::IpcSecurityTestUtil::PwnMessageReceived(
         web_rfh->GetProcess()->GetChannel(),
-        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(), 1,
+        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                        kRequestIdNotPreviouslyUsed,
                                         embedder_isolated_origin_msg));
     web_process_killed.Wait();
     SetBrowserClientForTesting(old_client);
   }
 
   // Web processes cannot make XHRs with invalid Origin headers.
   NavigateToURL(shell(), web_url);
   {
     RenderProcessHostWatcher web_process_killed(
         web_rfh->GetProcess(),
         RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
     IPC::IpcSecurityTestUtil::PwnMessageReceived(
         web_rfh->GetProcess()->GetChannel(),
-        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(), 1,
+        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                        kRequestIdNotPreviouslyUsed,
                                         invalid_origin_msg));
     web_process_killed.Wait();
   }
 
   // Web processes cannot make XHRs with invalid scheme Origin headers.
   NavigateToURL(shell(), web_url);
   {
     RenderProcessHostWatcher web_process_killed(
         web_rfh->GetProcess(),
         RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
     IPC::IpcSecurityTestUtil::PwnMessageReceived(
         web_rfh->GetProcess()->GetChannel(),
-        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(), 1,
+        ResourceHostMsg_RequestResource(web_rfh->GetRoutingID(),
+                                        kRequestIdNotPreviouslyUsed,
                                         invalid_scheme_origin_msg));
     web_process_killed.Wait();
   }
 }
 
+// Renderer process should not be able to create multiple requests with the same
+// id.
+IN_PROC_BROWSER_TEST_F(SecurityExploitBrowserTest, InvalidRequestId) {
+  // Existing loader in pending_loaders_.
+  TryCreateDuplicateRequestIds(shell(), false);
+  // Existing loader in blocked_loaders_map_.
+  TryCreateDuplicateRequestIds(shell(), true);
+}
+
 }  // namespace content