947592 - (CVE-2014-1487) Cross-origin information disclosure with error message of Web Workers

Status: VERIFIED FIXED

(Core :: DOM: Workers, defect)

Description

[Masato Kinugawa]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:25.0) Gecko/20100101 Firefox/25.0 (Beta/Release)
Build ID: 20131112160018

Steps to reproduce:

1. Go to http://vulnerabledoma.in/fx_worker.html and click the button.
2. If you are Twitter user, you can see the following message:

Failed to load script: https://twitter.com/[USERNAME]/lists

If not:

Failed to load script: https://twitter.com/login?redirect_after_login=%2Flists%2F in error message.

This means that Firefox leaks redirect information to unrelated site.


Actual results:

Firefox includes redirect information in error message.


Expected results:

Firefox should not include redirect information in error message.

Comment 1

[Daniel Veditz [:dveditz]]

calling this sec-high based on information disclosure... you might be able to get oauth tokens and the like this way.

Comment 2

[Daniel Veditz [:dveditz]]

Is this a regression or have we always leaked this?

Comment 3

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

I doubt this is a regression.

Comment 4

[Andrew McCreight [:mccr8]]

Attachment: Cross-origin information disclosure with error message of Web Workers (test)

Here's a rough mochitest for this problem.  The base HTML page, test_ErrorXO.html, starts
a worker with evil_redirect.sjs, which it is same origin with.  evil_redirect.sjs then
redirects to redirect.sjs, which it is not same origin with, which in turn redirects to
my_private_url, which it is same origin with.  This fails, and test_ErrorXO.html gets
back the url, which contains my_private_url.

Comment 5

[Andrew McCreight [:mccr8]]

I got into a bit of a wild goose chase because I'm not familiar with this code, but I think in the end the problem is pretty simple: near the end of ScriptLoaderRunnable::OnStreamCompleteInternal, it does some security checks to make sure that the load and channel principals match up, but they don't, so it returns NS_ERROR_DOM_BAD_URI.  But unfortunately, ReportLoadError has no case for that, so we just end up in the default case, which includes the url.

It seems like the best idea to me is to have a default case that doesn't return any interesting information, and maybe explicitly pick a few error cases that look like they can't be security-related and have those use the current default code.  Or something like that, I don't know how sensitive pages are to error messages like this.  Any thoughts?

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: Streamline ReportLoadError.

Comment 7

[Andrew McCreight [:mccr8]]

green L64 debug try run: https://tbpl.mozilla.org/?tree=Try&rev=404053c2911c

I looked over the ways in which loading could fail, and nothing seemed super important to convey to the page, so I didn't whitelist anything else.  I'm not sure how regression-sensitive this is.  I'm guessing not very.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 8364485 [details] [diff] [review]
Streamline ReportLoadError.

Review of attachment 8364485 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for diving in here!

::: dom/workers/ScriptLoader.cpp
@@ +875,5 @@
>        Throw(aCx, aLoadResult);
>        break;
>  
>      default:
> +      JS_ReportError(aCx, "Load error");

Can we just keep the old message without the url? Having the error code in the message could be useful for debugging.

Comment 9

[Andrew McCreight [:mccr8]]

Sure.  I suppose it doesn't reveal any important cross-origin information.

Comment 10

[Andrew McCreight [:mccr8]]

Attachment: Streamline ReportLoadError.

Addressed review comment.

Comment 11

[Andrew McCreight [:mccr8]]

Comment on attachment 8364692 [details] [diff] [review]
Streamline ReportLoadError.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?  Probably not too difficult.  It shows that we're revealing URL information when we shouldn't, which suggests a problem during loading of workers.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? No.

Which older supported branches are affected by this flaw? All.

If not all supported branches, which bug introduced the flaw?

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? The backports should be trivial.

How likely is this patch to cause regressions; how much testing does it need?  This does change web-exposed behavior, but only on workers, which are not super widely used, and only in some odd error configurations, so it doesn't seem so risky, but it could cause problems.

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Comment on attachment 8364692 [details] [diff] [review]
Streamline ReportLoadError.

sec-approval+ for trunk. Nominate for branches (Aurora, Beta, ESR24) ASAP as we're getting to last beta. If this wasn't sec-high, we wouldn't take this right now.

Comment 13

[Andrew McCreight [:mccr8]]

Comment on attachment 8364692 [details] [diff] [review]
Streamline ReportLoadError.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): pretty old
User impact if declined: sec-high bug
Testing completed (on m-c, etc.): try run
Risk to taking this patch (and alternatives if risky): fairly low, it does affect behavior slightly that pages can see, but only for a not-widely-used feature in an uncommon error condition
String or IDL/UUID changes made by this patch: none, or at least, I assume there's no localization of internally generated error messages

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: sec bug
Fix Landed on Version: 29
Risk to taking this patch (and alternatives if risky): see above
String or UUID changes made by this patch: see above

Comment 14

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1c3f5cb23af7

Comment 15

[Andrew McCreight [:mccr8]]

https://hg.mozilla.org/releases/mozilla-beta/rev/53ee1bb74686
https://hg.mozilla.org/releases/mozilla-aurora/rev/13f68c5b1ba2

I don't have an esr24 handy, so I'll see if Ryan lands it tomorrow.

Comment 16

[Carsten Book [:Tomcat]]

landed on central https://hg.mozilla.org/mozilla-central/rev/1c3f5cb23af7

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr24/rev/a0b4608c0802
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/68e3fff580e7
https://hg.mozilla.org/releases/mozilla-b2g18/rev/118c789809a0

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/118c789809a0

Comment 19

[Bogdan Maris, Desktop QA]

Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20140126 Firefox/24.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:24.0) Gecko/20140126 Firefox/24.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20140126 Firefox/24.0
Firefox 24.2.0ESR pre - buildID: 20140126000501

Mozilla/5.0 (X11; Linux i686; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:27.0) Gecko/20100101 Firefox/27.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:27.0) Gecko/20100101 Firefox/27.0
Firefox 27.0RC - buildID: 20140127194636

Mozilla/5.0 (X11; Linux i686; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:28.0) Gecko/20100101 Firefox/28.0
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:28.0) Gecko/20100101 Firefox/28.0
Aurora 28.0a2 - buildID: 20140127004003

Mozilla/5.0 (X11; Linux i686; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.9; rv:29.0) Gecko/20100101 Firefox/29.0
Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:29.0) Gecko/20100101 Firefox/29.0
Nightly 29.0a1 - buildID: 20140127072047

Verified as fixed on builds from above using Windows 7x64, Ubuntu 12.04x32 and Mac OS X 10.9.1.