[Backport] CVE-2018-16074

Avoid sharing process for blob URLs with null origin.

Previously, when a frame with a unique origin, such as from a data
URL, created a blob URL, the blob URL looked like blob:null/guid and
resulted in a site URL of "blob:" when navigated to.  This incorrectly
allowed all such blob URLs to share a process, even if they were
created by different sites.

This CL changes the site URL assigned in such cases to be the full
blob URL, which includes the GUID.  This avoids process sharing for
all blob URLs with unique origins.

This fix is conservative in the sense that it would also isolate
different blob URLs created by the same unique origin from each other.
This case isn't expected to be common, so it's unlikely to affect
process count.  There's ongoing work to maintain a GUID for unique
origins, so longer-term, we could try using that to track down the
creator and potentially use that GUID in the site URL instead of the
blob URL's GUID, to avoid unnecessary process isolation in scenarios
like this.

Note that as part of this, we discovered a bug where data URLs aren't
able to script blob URLs that they create: https://crbug.com/865254.
This scripting bug should be fixed independently of this CL, and as
far as we can tell, this CL doesn't regress scripting cases like this
further.

Bug: 863623
Change-Id: I861330de193039ac9f6ef9039e7cd9a2c3d3d383
Reviewed-on: https://chromium-review.googlesource.com/1142389
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

2 files changed, 71 insertions, 1 deletions

diff --git a/chromium/content/browser/site_instance_impl.cc b/chromium/content/browser/site_instance_impl.cc
index d5f3fcc1424..f4584ea8c32 100644
--- a/chromium/content/browser/site_instance_impl.cc
+++ b/chromium/content/browser/site_instance_impl.cc
@@ -427,10 +427,30 @@ GURL SiteInstance::GetSiteForURL(BrowserContext* browser_context,
   // This is useful for cases like file URLs.
   if (!origin.unique()) {
     // Prefer to use the scheme of |origin| rather than |url|, to correctly
-    // cover blob: and filesystem: URIs (see also https://crbug.com/697111).
+    // cover blob:file: and filesystem:file: URIs (see also
+    // https://crbug.com/697111).
     DCHECK(!origin.scheme().empty());
     return GURL(origin.scheme() + ":");
   } else if (url.has_scheme()) {
+    // In some cases, it is not safe to use just the scheme as a site URL, as
+    // that might allow two URLs created by different sites to to share a
+    // process.  See https://crbug.com/863623.
+    //
+    // TODO(alexmos,creis): This should eventually be expanded to certain other
+    // schemes, such as data: and file:.
+    if (url.SchemeIsBlob()) {
+      // We get here for blob URLs of form blob:null/guid.  Use the full URL
+      // with the guid in that case, which isolates all blob URLs with unique
+      // origins from each other.  Remove hash from the URL, since
+      // same-document navigations shouldn't use a different site URL.
+      if (url.has_ref()) {
+        GURL::Replacements replacements;
+        replacements.ClearRef();
+        url = url.ReplaceComponents(replacements);
+      }
+      return url;
+    }
+
     DCHECK(!url.scheme().empty());
     return GURL(url.scheme() + ":");
   }
diff --git a/chromium/content/browser/site_per_process_browsertest.cc b/chromium/content/browser/site_per_process_browsertest.cc
index 92bff576b82..c4095dce2ec 100644
--- a/chromium/content/browser/site_per_process_browsertest.cc
+++ b/chromium/content/browser/site_per_process_browsertest.cc
@@ -13800,4 +13800,54 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
       popup_subframe->current_frame_host()->GetSiteInstance()->GetSiteURL());
 }
 
+// Ensure that when two cross-site frames have subframes with unique origins,
+// and those subframes create blob URLs and navigate to them, the blob URLs end
+// up in different processes. See https://crbug.com/863623.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       TwoBlobURLsWithNullOriginDontShareProcess) {
+  GURL main_url(embedded_test_server()->GetURL(
+      "a.com", "/navigation_controller/page_with_data_iframe.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  FrameTreeNode* root = web_contents()->GetFrameTree()->root();
+  FrameTreeNode* subframe = root->child_at(0);
+
+  // Create a blob URL in the subframe, and navigate to it.
+  TestNavigationObserver observer(shell()->web_contents());
+  std::string blob_script =
+      "var blob = new Blob(['foo'], {type : 'text/html'});"
+      "var url = URL.createObjectURL(blob);"
+      "location = url;";
+  EXPECT_TRUE(ExecuteScript(subframe, blob_script));
+  observer.Wait();
+  RenderFrameHostImpl* subframe_rfh = subframe->current_frame_host();
+  EXPECT_TRUE(subframe_rfh->GetLastCommittedURL().SchemeIsBlob());
+
+  // Open a cross-site popup and repeat these steps.
+  GURL popup_url(embedded_test_server()->GetURL(
+      "b.com", "/navigation_controller/page_with_data_iframe.html"));
+  Shell* new_shell = OpenPopup(root, popup_url, "");
+  FrameTreeNode* popup_root =
+      static_cast<WebContentsImpl*>(new_shell->web_contents())
+          ->GetFrameTree()
+          ->root();
+  FrameTreeNode* popup_subframe = popup_root->child_at(0);
+
+  TestNavigationObserver popup_observer(new_shell->web_contents());
+  EXPECT_TRUE(ExecuteScript(popup_subframe, blob_script));
+  popup_observer.Wait();
+  RenderFrameHostImpl* popup_subframe_rfh =
+      popup_subframe->current_frame_host();
+  EXPECT_TRUE(popup_subframe_rfh->GetLastCommittedURL().SchemeIsBlob());
+
+  // Ensure that the two blob subframes don't share a process or SiteInstance.
+  EXPECT_NE(subframe->current_frame_host()->GetSiteInstance(),
+            popup_subframe->current_frame_host()->GetSiteInstance());
+  EXPECT_NE(
+      subframe->current_frame_host()->GetSiteInstance()->GetProcess(),
+      popup_subframe->current_frame_host()->GetSiteInstance()->GetProcess());
+  EXPECT_NE(
+      subframe->current_frame_host()->GetSiteInstance()->GetSiteURL(),
+      popup_subframe->current_frame_host()->GetSiteInstance()->GetSiteURL());
+}
+
 }  // namespace content