Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15502 closed defect (fixed)

URL.createObjectURL() considered harmful

Reported by: mikeperry Owned by: arthuredelstein
Priority: High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-linkability, tbb-newnym, tbb-4.5-alpha, TorBrowserTeam201504R, MikePerry201504R
Cc: gk, brade, mcs, arthuredelstein Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Blobs are a mechanism for creating temporary files that live in the browser and can optionally be assigned a random GUID that can be accessed via the blob: scheme.

Unfortunately, this has several bad consequences for TBB:

  1. blob: URIs are whitelisted in NoScript
  2. blob: URIs survive New Identity
  3. blob: URIs are not isolated by top-level domain

I think this is tricky to exploit to get arbitrary scripts to run, because you already need scripts enabled to create these things. They are also not great to use as a tracking vector, because the GUID you get is randomly assigned.

However, they still deeply concern me because if you want to keep track of a short list of users, you can create blob uris for them, record those GUIDS, and cycle through this list of GUIDs for every user who visits any site.

Here's an example blob URI creation script that gives you a blob uri that you can throw in the URL bar. It will then execute scripts (pop up an alert) even if you have instructed NoScript to disable scripts globally:
https://people.torproject.org/~mikeperry/transient/tests/blob-uri-creation.html

You can also use the resulting URI to test and see that it survives New Identity.

This ticket probably needs several child tickets to deal with the various issues here. Or we could just simply drop support for the URI feature of the Blob APIs. It seems rather obscure and unnessary, since you can use these things as normal JS objects just fine without them being URIs.

Child Tickets

Change History (26)

comment:1 Changed 4 years ago by mikeperry

Issue #2 (surviving New Identity) is not as bad as I originally thought. According to https://developer.mozilla.org/en-US/docs/Web/API/URL/createObjectURL, this blob URI is only supposed to persist while the document that created it is still valid. However, there appears to be a race condition or delay here where the URI persists for longer than that. I was able to continue using it after New Identity for some time (about 30 seconds or so) before it stopped working.

comment:2 in reply to:  description Changed 4 years ago by gk

Cc: gk added

Replying to mikeperry:

Here's an example blob URI creation script that gives you a blob uri that you can throw in the URL bar. It will then execute scripts (pop up an alert) even if you have instructed NoScript to disable scripts globally:
https://people.torproject.org/~mikeperry/transient/tests/blob-uri-creation.html

Interesting, but setting the security slider to "high" does not let the blob: URI execute it seems. Nevertheless, this is pretty scary. I think the safest for 4.5 is to just disable the support for that scheme. We could then think about handling all the related issues properly.

comment:3 Changed 4 years ago by mcs

Cc: brade mcs added

This is definitely a scary feature. Did Mozilla really intend for blob: URIs created in document A to be accessible from document N? I wonder what the use case is? I agree with gk that disabling support for createObjectURL() is a good idea (I cannot imagine that it is widely used).

comment:4 Changed 4 years ago by arthuredelstein

Cc: arthuredelstein added

comment:5 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201504 added; TorBrowserTeam201503 removed

comment:6 Changed 4 years ago by mikeperry

Current goal for 4.5 is to make this preffed off by default, and explain to users in the release notes to be on the lookout for image editing/upload support and other related file activities being broken on sites.

Perhaps we should also have an error console logline here, too (which ideally logs the source script, like the canvas logging does).

comment:7 Changed 4 years ago by mikeperry

The ideal solution is to scope blob URIs to the first party domain if we can. If that is simple, we can go that route. But something tells me the scope of these beasts is going to be hard to control (since they are just dumb guids without any other origin).

comment:8 Changed 4 years ago by mikeperry

Owner: changed from tbb-team to arthuredelstein
Status: newassigned

Arthur said he'd investigate this. Assigning to him for now.

comment:9 Changed 4 years ago by mikeperry

Summary: Blob URIs considered harmfulURI.createObjectURL() considered harmful

Unfortunately, there's another case where this will bite us. The mediasource: scheme was created alongside VP9, and is currently used by youtube to play VP9 videos. mediasource URIs contain javascript handlers created by the MediaSource API, and are created by URI.createObject():
https://html5-mediasource-api.googlecode.com/svn/tags/0.1/draft-spec/mediasource-draft-spec.html#examples

It's not clear if URI.createObject() in that example is a typo for URL.createObjectURL(), or another API. Firefox 37 does not have a URI.createObject() or a URL.createObject().

MediaSource support is currently present but disabled by default in Firefox 31. You need to set the prefs 'media.mediasource.enabled' and 'media.mediasource.webm.enabled' to true in order for mediasource: URIs to be created. This means we may be able to get away with disabling URI.createObjectURL() for now, but once we hit FF38-ESR, we'll need to enable+isolate it, or Youtube will break.

comment:10 Changed 4 years ago by mikeperry

Summary: URI.createObjectURL() considered harmfulURL.createObjectURL() considered harmful

comment:11 Changed 4 years ago by arthuredelstein

Keywords: TorBrowserTeam201504R added; TorBrowserTeam201504 removed
Status: assignedneeds_review

comment:12 Changed 4 years ago by gk

Hm.. what about the second point above: surviving New Identity?

comment:13 in reply to:  12 Changed 4 years ago by arthuredelstein

Replying to gk:

Hm.. what about the second point above: surviving New Identity?

Oops -- here's a patch for that.

https://github.com/arthuredelstein/torbutton/commit/2568365dfa0ce8ebc2f5f430b4c752ca57b2fc97

comment:14 Changed 4 years ago by mikeperry

Keywords: MikePerry201504R added

comment:15 Changed 4 years ago by mikeperry

The tor-browser patch looks mostly ok to me, though I am a little worried about the use of nsContentUtils::GetDocumentFromCaller() in ThirdPartyUtil::GetFirstPartyHostFromCaller(). It is reminding me of #13027. We ultimately discovered that WebWorkers were given the correct Javascript context after creation, but can we explicitly test WebWorkers to ensure they can't access blob uris from different first party domains as well, just to be sure? Probably also wise to make this an actual in-tree test to ensure that it doesn't change on us in ff38-esr.

I think it might also be helpful to have mcs+brade to weigh in on this approach.

I filed #15703 to remind us about mediasource: and associated tests in ff38-esr.

comment:16 Changed 4 years ago by mikeperry

While testing the Torbutton piece, I noticed that about:memory was still reporting many page objects still present after New Identity. I couldn't manage to catch any blob: urls still alive in there, but it was enough to worry me.

I reviewed the garbage collector source in Firefox and it was a complete nightmare due to many layers of timers, incremental GC and CC heuristics, and other performance hacks. It turns out even calling the garbageCollect() call like you did only schedules a GC call 4-10 seconds later, at best. We have to jump through all sorts of hoops to get the thing to run immediately. I think this commit finally does the trick:
https://gitweb.torproject.org/mikeperry/torbutton.git/commit/?h=15502&id=accb4b887b4427221358fc8c17bbbdf85e77467c

I tested this by setting my homepage to about:memory, so I could click the 'Measure' button as fast as possible after New Identity.

Last edited 4 years ago by mikeperry (previous) (diff)

comment:17 in reply to:  15 Changed 4 years ago by arthuredelstein

Replying to mikeperry:

The tor-browser patch looks mostly ok to me, though I am a little worried about the use of nsContentUtils::GetDocumentFromCaller() in ThirdPartyUtil::GetFirstPartyHostFromCaller(). It is reminding me of #13027. We ultimately discovered that WebWorkers were given the correct Javascript context after creation, but can we explicitly test WebWorkers to ensure they can't access blob uris from different first party domains as well, just to be sure? Probably also wise to make this an actual in-tree test to ensure that it doesn't change on us in ff38-esr.

Good idea. I'll work on this today.

comment:18 in reply to:  15 Changed 4 years ago by mcs

Replying to mikeperry:

The tor-browser patch looks mostly ok to me, though I am a little worried about the use of nsContentUtils::GetDocumentFromCaller() in ThirdPartyUtil::GetFirstPartyHostFromCaller(). It is reminding me of #13027. We ultimately discovered that WebWorkers were given the correct Javascript context after creation, but can we explicitly test WebWorkers to ensure they can't access blob uris from different first party domains as well, just to be sure? Probably also wise to make this an actual in-tree test to ensure that it doesn't change on us in ff38-esr.

I think it might also be helpful to have mcs+brade to weigh in on this approach.

The use of nsContentUtils::GetDocumentFromCaller() caught our eye as well since we were not aware that such a call existed. It is only used in a couple of unimportant places in the Mozilla code, but Kathy and I think it will do the right thing as long as there is a JS context in the call stack. It would be nice to use a different approach, but it seems like the BlobURI implementation is really designed without "same origin" or other concepts in mind. I wonder if anyone at Mozilla did a security review of the design.

comment:19 Changed 4 years ago by arthuredelstein

Mike, Mark and Kathy -- you were all right to be worried about GetDocumentFromCaller. I wrote tests, here:
https://github.com/arthuredelstein/tor-browser/commit/e5cef7f72932f3c5eb54da4bf97b8886f85c846a
and, embarrassingly, I found out my patch does not properly isolate blob URLs created or read inside Web Workers.

I looked into how to fix this patch, but the Web Worker case is quite complex. Also I feel much less comfortable with GetDocumentFromCaller() now that it's already failed once. So for now (for Firefox 31) I would be in favor of disabling blob URLs in content. Here's a patch that does that:
https://github.com/arthuredelstein/tor-browser/commit/dfbd283c17225d79e1ff82bb933c59a77853ddf3

(I'll keep looking at how to write a different patch that isolates blob URLs per url bar domain without any stupid tricks like GetDocumentFromCaller.)

comment:20 in reply to:  19 Changed 4 years ago by mcs

Replying to arthuredelstein:

I looked into how to fix this patch, but the Web Worker case is quite complex. Also I feel much less comfortable with GetDocumentFromCaller() now that it's already failed once. So for now (for Firefox 31) I would be in favor of disabling blob URLs in content. Here's a patch that does that:
https://github.com/arthuredelstein/tor-browser/commit/dfbd283c17225d79e1ff82bb933c59a77853ddf3

The patch looks OK, at least based on my reading of https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings#ChromeOnly

I assume [ChromeOnly] works correctly since it is fairly widely used by Mozilla.

comment:21 Changed 4 years ago by mikeperry

Won't applying [ChromeOnly] to the whole URL constructor also prevent access to the URLUtils portions of the URL API (https://developer.mozilla.org/en-US/docs/Web/API/URL)? Those are benign, and likely used elsewhere. We really want to keep this focused only on createObjectURL()..

It might also be nice to have a console log message recording when/where createObjectURL() was called, so we can more easily inspect if it is what is breaking some random site..

comment:22 Changed 4 years ago by arthuredelstein

Here's a new version of the GetDocumentFromCaller patch:
https://github.com/arthuredelstein/tor-browser/commit/0d67ab406bdd3cf095802cb25c081641aa1f0bcc

This is the same as the previous patch, but with code added to dom/workers/URL.cpp, in URL::CreateObjectURL. When this method is called in a non-chrome web worker, a message is logged to the console, and separately, an exception is thrown. As we discussed, this is a stop-gap until we find a better way to isolate blob URLs to first party domain for both main-thread content scripts and web workers.

comment:23 Changed 4 years ago by arthuredelstein

I have now added a commit with regression tests to this branch:
https://github.com/arthuredelstein/tor-browser/commit/f4f2caa26dd2d15e8f99d0a1357361da43e4fd2f

comment:24 Changed 4 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

Ok, I think we're good to go on this one! Merged. Thanks, Arthur!

comment:25 in reply to:  16 ; Changed 3 years ago by cypherpunks

Severity: Normal

Replying to mikeperry:

While testing the Torbutton piece, I noticed that about:memory was still reporting many page objects still present after New Identity. I couldn't manage to catch any blob: urls still alive in there, but it was enough to worry me.

blob: URIs still survive New Identity. Is this OK?

comment:26 in reply to:  25 Changed 3 years ago by gk

Replying to cypherpunks:

Replying to mikeperry:

While testing the Torbutton piece, I noticed that about:memory was still reporting many page objects still present after New Identity. I couldn't manage to catch any blob: urls still alive in there, but it was enough to worry me.

blob: URIs still survive New Identity. Is this OK?

I don't think so. Do you mind opening a ticket and add steps to reproduce your findings? Commenting on an old and closed ticket risks that folks are missing your issue.

Note: See TracTickets for help on using tickets.