New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users

Issue metadata

Status: Fixed
Owner:
OOO until Feb 20
Closed: Jul 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 863623: Security: Blob URL created from Data URL shares same process despite creator being cross-site

Reported by s.h.h.n....@gmail.com, Jul 13 2018

Issue description

VULNERABILITY DETAILS
When a Blob URL is created from Data URL, it effectively has Opaque origin. But Site Isolation treat 2 opaque origins as same-site and commit it to same process when they are blob:null/. This is bad because those can be created from 2 different sites and may contain secret.

VERSION
Chrome Version: 67.0.3396.99 + stable
Operating System: macOS High Sierra 10.13.3

REPRODUCTION CASE
1. Go to https://shhnjk.azurewebsites.net/ISO_blob_data.html
2. Click on the button and observe that Blob URL created by process belongs to test.shhnjk.com and Blob URL created by process belongs to shhnjk.azurewebsites.net are now in the same process.
 

Comment 1 by est...@chromium.org, Jul 14 2018

Components: Internals>Sandbox>SiteIsolation
Labels: Security_Severity-Medium M-67 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-1
Owner: creis@chromium.org
Status: Assigned (was: Unconfirmed)
Over to Charlie for triage.

Comment 2 by sheriffbot@chromium.org, Jul 15 2018

Project Member
Labels: Security_Impact-Stable

Comment 3 by creis@chromium.org, Jul 16 2018

Cc: alex...@chromium.org
Looks like we might be sharing processes for a unique origin?  (OOPIFs are normally put into an existing process for their site if possible, but obviously that shouldn't apply to unique origins.)  I'll try to confirm in a debugger this afternoon.

Comment 4 by dcheng@chromium.org, Jul 16 2018

I wonder if this could benefit from some of the work we've been talking about to track the creator origin / associate origins and URLs more strongly.

Comment 5 by alex...@chromium.org, Jul 16 2018

Checking on what's returned from GetSiteInstanceForNavigation, it looks like we end up placing the blob URL into a SiteInstance with site URL of "blob:".  That doesn't seem right -- presumably we should've left the blob URL in the same SiteInstance and process as the data URL that created it.

That explains why the second blob URL ended up incorrectly sharing the first blob URL's process - it also ends up needing a "blob:" SiteInstance, and it either reuses the first SiteInstance (if the second tab is in the same BrowsingInstance, which I believe is true in this repro), or it creates a new SiteInstance and reuses the first blob: SiteInstance's process, due to 
our subframe process reuse policy.  For the latter, the first process would be considered compatible for reuse since it's locked to the same "site" of "blob:".

Comment 6 by alex...@chromium.org, Jul 17 2018

Cc: -alex...@chromium.org creis@chromium.org dcheng@chromium.org
Owner: alex...@chromium.org
I'll try to help out with this.  After chatting with Charlie, we think a reasonable short-term fix might be to also append the GUID portion of the blob URL to the site URL in cases where the blob URL has a unique origin, so that blobs created from different sites wouldn't share a process.

Ideally, we'd figure out that the blob URL was created by the data URL currently in the subframe and reuse that SiteInstance.  We could do this if we had GUIDs associated with unique origins, which I hear dcheng@ is working on, though we'd need additional plumbing to pass that information along with the dest_url of "blob:null/{guid}", since the latter is all we currently have in DetermineSiteInstanceForURL(), and that seems nontrivial.

I've also confirmed that you get "blob:null/{guid}" when creating a blob URL from a sandboxed iframe, so this is another way to get to this bug without using data URLs.

Another thing to consider here is whether we should disallow process reuse across processes locked to a site without a host, in case the URLs that resulted in these site URLs came from different sites.  This would affect site URLs like "blob:", "data:", "about:", "file:", "content:", etc.   This wouldn't be enough to fix this bug within the same BrowsingInstance, since we'd still find an existing SiteInstance when we look up something like "blob:".  OTOH, maybe the better thing to move toward is to make site URLs for these cases more granular (e.g., include path in site URL for a file URL, use full data URL as site URL), and keep the sharing where it doesn't pose any risks (e.g., for "about:").

Comment 7 by creis@chromium.org, Jul 18 2018

I agree with the plan to make site URLs more granular for unique origins.  We're already planning that for one case in issue 780770, and it will help with data URLs in  issue 863069  as well.

Comment 8 by alex...@chromium.org, Jul 18 2018

Status: Started (was: Assigned)
I've got a CL started for using the full blob URL as site URL when the blob URL's origin is unique - https://chromium-review.googlesource.com/c/chromium/src/+/1142389

Comment 9 by bugdroid1@chromium.org, Jul 19 2018

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b1f87486936ca0d6d9abf4d3b9b423a9c976fd59

commit b1f87486936ca0d6d9abf4d3b9b423a9c976fd59
Author: Alex Moshchuk <alexmos@chromium.org>
Date: Thu Jul 19 01:51:51 2018

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: Ib50407adbba3d5ee0cf6d72d3df7f8d8f24684ee
Reviewed-on: https://chromium-review.googlesource.com/1142389
Commit-Queue: Alex Moshchuk <alexmos@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576318}
[modify] https://crrev.com/b1f87486936ca0d6d9abf4d3b9b423a9c976fd59/content/browser/site_instance_impl.cc
[modify] https://crrev.com/b1f87486936ca0d6d9abf4d3b9b423a9c976fd59/content/browser/site_instance_impl_unittest.cc
[modify] https://crrev.com/b1f87486936ca0d6d9abf4d3b9b423a9c976fd59/content/browser/site_per_process_browsertest.cc

Comment 10 by alex...@chromium.org, Jul 20 2018

Status: Fixed (was: Started)
Verified r576318 in Mac Canary 69.0.3497.0 using s.h.h.n.j.k@'s repro steps and URL.  The two blob subframes don't share a process anymore.  One side effect of the change is that the task manager entries now show up as "Subframe: blob:null/[guid]" instead of "Subframe: blob:".  The null/[guid] part may not really be worth showing to the user, but we can probably live with that for now.

Comment 11 by sheriffbot@chromium.org, Jul 21 2018

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 12 by awhalley@chromium.org, Jul 23 2018

Labels: reward-topanel

Comment 13 by awhalley@google.com, Jul 23 2018

Labels: -M-67 M-69

Comment 14 by s.h.h.n....@gmail.com, Jul 28 2018

Just FYI, this might be known issue but any website can spawn hundreds of process. This is even easy after this patch because now all blob url with null origin are treat as cross-site.

PoC
https://test.shhnjk.com/max_iso.html

Code:
<body>
<script>
 setInterval(() => {document.body.appendChild(document.createElement("iframe")).src = "data:text/html,<script>location.href = URL.createObjectURL(new Blob(['test'], {type : 'text/html'}));<\/script>";},1);
</script>
</body>

This hangs Chrome as well as other application running in the OS. Maybe it's better to provide some detection against this technique and allow user to stop page's script as you do for infinity for-loop scripts.

Comment 15 by creis@chromium.org, Jul 30 2018

Comment 14: Thanks, yes, that's a known issue (issue 841984).  Agreed that this might make it easier.

Comment 16 by awhalley@chromium.org, Jul 30 2018

Labels: -reward-topanel reward-unpaid reward-3000
*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

Comment 17 by awhalley@chromium.org, Jul 30 2018

$3,000 for this report. Thanks as always!

Comment 18 by awhalley@chromium.org, Jul 30 2018

Labels: -reward-unpaid reward-inprocess

Comment 19 by sheriffbot@chromium.org, Aug 3

Project Member
Labels: Merge-Request-69

Comment 20 by sheriffbot@chromium.org, Aug 3

Project Member
Labels: -Merge-Request-69 Merge-Review-69 Hotlist-Merge-Review
This bug requires manual review: M69 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), kariahda@(iOS), cindyb@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 21 by gov...@chromium.org, Aug 3

Cl listed at #9 is already in M69 branch 3497 per comment #10. Is there any merge needed for M69? If not, pls remove "Merge-Review-69" label.

Comment 22 by alex...@chromium.org, Aug 3

Labels: -Hotlist-Merge-Review -Merge-Review-69
#21: Correct, r576318 initially appeared in 69.0.3497.0, so no need for a merge.

Comment 23 by gov...@chromium.org, Aug 3

Labels: Merge-Reject-69
Adding "Merge-Reject-69" label so sheriffbot again doesn't request a merge to M69.

Comment 24 by gov...@chromium.org, Aug 3

Labels: -Merge-Reject-69 Merge-Rejected-69

Comment 25 by awhalley@google.com, Aug 16

Labels: Release-0-M69

Comment 26 by awhalley@chromium.org, Sep 4

Labels: CVE-2018-16074 CVE_description-missing

Comment 27 by sheriffbot@chromium.org, Oct 27

Project Member
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment