Chromium Code Reviews Chromium Code Reviews
Issue 2387173005:
Lock down creation of "filesystem:chrome-extension://" URLs (Closed)
| 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 c95bb562d6bbe45c7bbdbe6b1d24f62d1ba76987..4661a5f042c64dd90f35a82e41006b5799a031f3 100644 |
| --- a/chrome/browser/chrome_security_exploit_browsertest.cc |
| +++ b/chrome/browser/chrome_security_exploit_browsertest.cc |
| @@ -12,6 +12,7 @@ |
| #include "chrome/browser/ui/tabs/tab_strip_model.h" |
| #include "chrome/test/base/in_process_browser_test.h" |
| #include "chrome/test/base/ui_test_utils.h" |
| +#include "content/common/fileapi/file_system_messages.h" |
| #include "content/common/fileapi/webblob_messages.h" |
| #include "content/public/browser/notification_observer.h" |
| #include "content/public/browser/notification_service.h" |
| @@ -69,6 +70,8 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest, |
| EXPECT_STREQ(status.c_str(), expected_status.c_str()); |
| } |
| +// A normal renderer process should not be able to create a |
| +// blob:chrome-extension:// resource. |
| IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest, |
| CreateBlobInExtensionOrigin) { |
| ui_test_utils::NavigateToURL( |
| @@ -113,3 +116,90 @@ IN_PROC_BROWSER_TEST_F(ChromeSecurityExploitBrowserTest, |
| GURL("blob:" + target_origin + "/" + blob_path), blob_id)); |
| crash_observer.Wait(); // If the process is killed, this test passes. |
| } |
| + |
| +// A normal renderer process should not be able to create a |
| +// filesystem:chrome-extension:// 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(); |
| + |
| + // All these are attacker controlled values. The UUID is arbitrary. |
| + std::string blob_id = "2ce53a26-0409-45a3-86e5-f8fb9f5566d8"; |
| + std::string blob_type = "text/html"; |
| + std::string blob_contents = |
| + "<html><body>pwned.<script>chrome.extensions</script>"; |
| + std::string blob_path = "5881f76e-10d2-410d-8c61-ef210502acfd"; |
|
Charlie Reis
2016/10/04 22:19:04
Some cleanup possible here.
ncarter (slow)
2016/10/04 23:09:36
Done.
|
| + |
| + // Target the bookmark manager extension. |
| + std::string target_origin = |
| + "chrome-extension://eemcgdkfndhakfknompkggombfjjjeno"; |
| + GURL target_url = |
| + GURL("filesystem:" + target_origin + "/temporary/exploit.html"); |
| + |
| + std::vector<storage::DataElement> data_elements(1); |
| + data_elements[0].SetToBytes(blob_contents.c_str(), blob_contents.size()); |
|
Charlie Reis
2016/10/04 22:19:04
Move below.
ncarter (slow)
2016/10/04 23:09:36
Done.
|
| + |
| + { |
| + content::RenderProcessHostWatcher crash_observer( |
| + rfh->GetProcess(), |
| + content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); |
| + |
| + IPC::IpcSecurityTestUtil::PwnMessageReceived( |
| + rfh->GetProcess()->GetChannel(), |
| + FileSystemHostMsg_Create(55, target_url, false, false, false)); |
| + |
| + // Wait for the child process to die as a side effect of the unexpected |
| + // FileSystemMsg_DidSucceed or FileSystemMsg_DidFail. |
| + crash_observer.Wait(); |
| + } |
| + |
| + // Reload the page. |
| + ui_test_utils::NavigateToURL(browser(), page_url); |
| + { |
| + content::RenderProcessHostWatcher crash_observer( |
| + rfh->GetProcess(), |
| + content::RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT); |
| + |
| + // Set up a blob ID and populate it with attacker-controlled value. These |
| + // two messages are allowed, because this data is not in any origin. |
| + IPC::IpcSecurityTestUtil::PwnMessageReceived( |
| + rfh->GetProcess()->GetChannel(), |
| + BlobStorageMsg_RegisterBlobUUID(blob_id, blob_type, "", |
| + std::set<std::string>())); |
| + IPC::IpcSecurityTestUtil::PwnMessageReceived( |
| + rfh->GetProcess()->GetChannel(), |
| + BlobStorageMsg_StartBuildingBlob(blob_id, data_elements)); |
| + |
| + // Write the blob into the file. If successful, this places an |
| + // attacker-controlled value in a resource on the extension origin. |
| + IPC::IpcSecurityTestUtil::PwnMessageReceived( |
| + rfh->GetProcess()->GetChannel(), |
| + FileSystemHostMsg_Write(56, target_url, blob_id, 0)); |
| + |
| + // Wait for the child process to die as a side effect of the unexpected |
| + // FileSystemMsg_DidWrite (if the exploit step succeeded) or |
| + // FileSystemMsg_DidFail (if we soft-failed the operation), or a renderer |
| + // kill (if we enforced the origin permission with a kill). |
| + crash_observer.Wait(); |
| + } |
| + |
| + // Make sure that the above IPCs did not succeed, and |target_url| gets a 404 |
| + // response. |
| + ui_test_utils::NavigateToURL(browser(), target_url); |
| + 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)); |
| + EXPECT_EQ( |
|
Charlie Reis
2016/10/04 22:19:04
Gate this (or the test) on --isolate-extensions mo
ncarter (slow)
2016/10/04 23:09:36
Done.
|
| + "\nYour file was not found\n\n" |
| + "It may have been moved or deleted.\n" |
| + "ERR_FILE_NOT_FOUND\n", |
| + body); |
| +} |
