browser_test that creates filesystem resources in extension origin

chrome/browser/chrome_security_exploit_browsertest.cc

Index: chrome/browser/chrome_security_exploit_browsertest.cc
diff --git a/chrome/browser/chrome_security_exploit_browsertest.cc b/chrome/browser/chrome_security_exploit_browsertest.cc
index cc840243d021422483d7396aa5faa7263d8dde71..177d3106ba90191369337e74435aa0825c59b2a2 100644
--- a/chrome/browser/chrome_security_exploit_browsertest.cc
+++ b/chrome/browser/chrome_security_exploit_browsertest.cc
@@ -5,6 +5,7 @@
 #include "base/command_line.h"
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/histogram_tester.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/singleton_tabs.h"
@@ -12,7 +13,6 @@
 #include "chrome/common/extensions/extension_process_policy.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
-#include "content/common/fileapi/webblob_messages.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/notification_types.h"
@@ -22,7 +22,6 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.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"
 
@@ -69,6 +68,8 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,
   EXPECT_STREQ(status.c_str(), expected_status.c_str());
 }
 
+// Extension isolation prevents a normal renderer process from being able to
+// create a "blob:chrome-extension://" resource.
 IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,
                        CreateBlobInExtensionOrigin) {
   // This test relies on extensions documents running in extension processes,
@@ -95,23 +96,100 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,
   std::string target_origin =
       "chrome-extension://eemcgdkfndhakfknompkggombfjjjeno";
 
-  std::vector<storage::DataElement> data_elements(1);
-  data_elements[0].SetToBytes(blob_contents.c_str(), blob_contents.size());
-
   // Set up a blob ID and populate it with attacker-controlled value. This
   // message is allowed, because this data is not in any origin.
-  IPC::IpcSecurityTestUtil::PwnMessageReceived(
-      rfh->GetProcess()->GetChannel(),
-      BlobStorageMsg_RegisterBlob(blob_id, blob_type, "", data_elements));
+  content::PwnMessageHelper::CreateBlobWithPayload(
+      rfh->GetProcess(), blob_id, blob_type, "", blob_contents);
 
   // This IPC should result in a kill because |target_origin| is not commitable
   // in |rfh->GetProcess()|.
+  base::HistogramTester histograms;
   content::RenderProcessHostWatcher crash_observer(
       rfh->GetProcess(),
       content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
-  IPC::IpcSecurityTestUtil::PwnMessageReceived(
-      rfh->GetProcess()->GetChannel(),
-      BlobHostMsg_RegisterPublicURL(
-          GURL("blob:" + target_origin + "/" + blob_path), blob_id));
-  crash_observer.Wait();  // If the process is killed, this test passes.
+
+  content::PwnMessageHelper::RegisterBlobURL(
+      rfh->GetProcess(), GURL("blob:" + target_origin + "/" + blob_path),
+      blob_id);
+
+  // If the process is killed, this test passes.
+  crash_observer.Wait();
+  histograms.ExpectUniqueSample("Stability.BadMessageTerminated.Content", 139,
+                                1);
+}
+
+// Extension isolation prevents a normal renderer process from being able to
+// create a "filesystem:chrome-extension://sdgkjaghsdg/temporary/" resource.
+IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest,
+                       CreateFilesystemURLInExtensionOrigin) {
+  GURL page_url =
+      embedded_test_server()->GetURL("a.root-servers.net", "/title1.html");
+  ui_test_utils::NavigateToURL(browser(), page_url);
+
+  content::RenderFrameHost* rfh =
+      browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+
+  // Block the renderer on operation that never completes, to shield it from
+  // recieving unexpected browser->renderer IPCs that might CHECK.

[alexmos, 2016/12/02 00:01:56]

nit: s/recieving/receiving/

[ncarter (slow), 2016/12/02 19:17:45]

Done.

+  rfh->ExecuteJavaScriptWithUserGestureForTests(
+      base::ASCIIToUTF16("var r = new XMLHttpRequest();"
+                         "r.open('GET', '/slow?99999', false);"
+                         "r.send(null);"
+                         "while (1);"));

[alexmos, 2016/12/02 00:01:56]

IIUC, we should never reach the "while (1)" as the sync XHR will block on the
send() above it.  Is this a safeguard of some sort?

[ncarter (slow), 2016/12/02 19:17:45]

I put this here as a proactive fallback -- I was worried about xhr completing
due to a network error, maybe while the test shuts down the testserver; these
IPCs are still in the pipe waiting to produce CHECK failures the moment that
happens.

I originally implemented this with just the while(1) but noticed that it ran
slowly, so added the xhr to prevent a busy core.

+
+  // JS code that the attacker would like to run in an extension process.
+  std::string payload = "<html><body>pwned.<script></script>";

[alexmos, 2016/12/02 00:01:56]

Did this intentionally not have the </body></html>? :)

[ncarter (slow), 2016/12/02 19:17:45]

Done.

+  std::string payload_type = "text/html";
+
+  // Target the bookmark manager extension.
+  std::string target_origin =
+      "chrome-extension://eemcgdkfndhakfknompkggombfjjjeno/";
+
+  // Set up a blob ID and populate it with the attacker-controlled payload.
+  // This is allowed, because this data is not in any origin;
+  // the UUID is arbitrary.
+  std::string blob_id = "2ce53a26-0409-45a3-86e5-f8fb9f5566d8";
+  content::PwnMessageHelper::CreateBlobWithPayload(rfh->GetProcess(), blob_id,
+                                                   payload_type, "", payload);
+
+  // Note: a well-behaved renderer would always send the following message here,
+  // but it's actually not necessary for the original attack to succeed, so we
+  // omit it. As a result there are some log warnings from the quota observer.
+  //
+  // IPC::IpcSecurityTestUtil::PwnMessageReceived(
+  //     rfh->GetProcess()->GetChannel(),
+  //     FileSystemHostMsg_OpenFileSystem(22, GURL(target_origin),
+  //                                      storage::kFileSystemTypeTemporary));
+
+  GURL target_url =
+      GURL("filesystem:" + target_origin + "temporary/exploit.html");
+
+  content::PwnMessageHelper::FileSystemCreate(rfh->GetProcess(), 23, target_url,
+                                              false, false, false);
+
+  // Write the blob into the file. If successful, this places an
+  // attacker-controlled value in a resource on the extension origin.
+  content::PwnMessageHelper::FileSystemWrite(rfh->GetProcess(), 24, target_url,
+                                             blob_id, 0);
+
+  // Now navigate to |target_url| in a new tab. It should not contain |payload|.
+  AddTabAtIndex(0, target_url, ui::PAGE_TRANSITION_TYPED);
+  content::WaitForLoadStop(browser()->tab_strip_model()->GetWebContentsAt(0));
+  rfh = browser()->tab_strip_model()->GetActiveWebContents()->GetMainFrame();
+  EXPECT_EQ(GURL(target_origin), rfh->GetSiteInstance()->GetSiteURL());
+  std::string body;
+  EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+      rfh, "window.domAutomationController.send(document.body.innerText);",
+      &body));
+  if (extensions::IsIsolateExtensionsEnabled()) {
+    EXPECT_EQ(
+        "\nYour file was not found\n\n"
+        "It may have been moved or deleted.\n"
+        "ERR_FILE_NOT_FOUND\n",
+        body);
+  } else {
+    // Without --isolate-extensions, the above steps must succeed, since
+    // unblessed extension frames are allowed in ordinary renderer processes.
+    EXPECT_EQ("pwned.", body);
+  }
 }