1245724 - (CVE-2016-1949) NPAPI-initiated network requests can be intercepted by service workers breaking plugin origin expectations

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[jasonpang2011]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/48.0.2564.97 Safari/537.36

Steps to reproduce:

Demo page: https://onesignal.com/serviceworker-flash.html

1. Visit the demo page, and see the video play normally on first page load.

2. Click the Register Service Worker button to register the service worker.

3. Refresh the page for the service worker to take control, and the player will give an error.

4. Force refresh the page without cache (Ctrl + Shift + R) and the player will not give an error.


Actual results:

 Video plays correctly when service worker is not in control. When service worker (even a simple worker) controls the page, the flash video does not play. 


Expected results:

Flash video should play correctly regardless of whether the service worker is in control. This demo service worker does nothing but print a line to the console (does not install onFetch handler or intercept network requests).

Comment 1

[Loic]

Interesting, it works with e10s enabled in Aurora/Nightly. Did you try?

Comment 2

[Ben Kelly [:bkelly, not reviewing]]

I'll look at this later today.

Comment 3

[jasonpang2011]

(In reply to Loic from comment #1)
> Interesting, it works with e10s enabled in Aurora/Nightly. Did you try?

Hi, confirmed! If I uncheck "Enable multi-process Nightly", the same flash error occurs, but if I leave "Enable multi-process Nightly" checked (as it is checked by default in Firefox Nightly), the video plays successfully without errors regardless of whether the service worker is in control.

Comment 4

[Andrew Overholt [:overholt]]

Thanks for confirming it's an e10s-only issue.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

The request that seems to be getting blocked is:

https://onesignal.com/sample-video.mp4
Host: onesignal.com
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
Accept-Language: en-US,en;q=0.5
Accept-Encoding: gzip, deflate, br
Referer: https://ssl.p.jwpcdn.com/player/v/7.2.4/jwplayer.flash.swf
Cookie: __cfduid=dd5a94d23d34f1976b543e7b7500af8481454686651
Connection: keep-alive

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Hide internal and hsts upgrade redirects from plugins. r=bsmedberg

This fixes the problem for me.  It seems the flash plugin gets confused if we expose the internal redirect to it.  Since internal redirects are internal, I propose we just hide these redirect callbacks from all plugins.

I'm going to try to write a mochitest for this next.

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

Attachment: wip test

Work in progress on the test.  I plan to mainly copy test_redirect_handling.html from the plugin directory.  I'll verify a plugin does not see a redirect from a service worker non-interception.

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8716453 [details] [diff] [review]
P1 Hide internal and hsts upgrade redirects from plugins. r=bsmedberg

I think I might be able to come up with a better patch here.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Make plugin redirect handling ignore internal redirects without a status code. r=bsmedberg

Alternate patch that makes the plugin code gracefully handle a redirect without a response status code.  The service worker internal redirects occur before the original channel has a response status, so GetResponseStatus() returns error in that case.

I'm not sure if this or the other patch is better.  With this patch the plugin may see some internal redirects if the original channel has a non-30x response status.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

Another alternative would be to still call the plugin redirect notify function, but pass it a status code of 0.  I can't find documentation for this function, so I don't know which would be best based on plugin expectations.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8716753 [details] [diff] [review]
P1 Make plugin redirect handling ignore internal redirects without a status  code. r=bsmedberg

Sorry for the flag spam.  From discussion with spec editors it seems we should not be intercepting these at all.  The spec itself is vague or missing this, though.

Comment 12

[Ben Kelly [:bkelly, not reviewing]]

Jake tells me this is actually a critical security problem:

10:04 AM <JakeA> It's a serious security hole if you can intercept flash's crossdomain.xml requests for instance
10:04 AM <JakeA> blows apart origin security
10:05 AM <wanderview> ok, seems reasonable
10:32 AM <wanderview> ah... we already try to block embed/object from service worker... we just do a crappy job of it
10:39 AM <JakeA> if crossdomain.xml is getting through, that's a high priority security fix
10:39 AM <JakeA> although there may be others. It's been a long time since I did Flash
10:40 AM <wanderview> I haven't seen that particular URL... is that something set within flash or on the <object> tag itself?
10:40 AM <JakeA> It's something Flash itself will request
10:40 AM <JakeA> It's the Flash version of CORS
10:41 AM <JakeA> it'll request that from the root of the origin, and if it returns XML in a particular way it'll expose all content on the origin
10:41 AM <JakeA> dunno if the requests are credentialed
10:41 AM <JakeA> http://www.adobe.com/devnet/adobe-media-server/articles/cross-domain-xml-for-streaming.html
10:41 AM <wanderview> ok, thanks

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Make plugin network requests bypass service worker interception. r=ehsan

I will write a test this afternoon.  Hopefully I can just improve our current <embed>/<object> test.

Comment 14

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Test that service workers do not intercept plugin network requests. r=ehsan

Comment 15

[(no longer active)]

Comment on attachment 8717097 [details] [diff] [review]
P2 Test that service workers do not intercept plugin network requests. r=ehsan

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

::: dom/workers/test/serviceworkers/fetch/plugin/plugins.html
@@ +1,2 @@
>  <!DOCTYPE html>
> +<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>

Please remove this (see below.)

@@ +55,5 @@
> +    is(interceptList.length, 1, "should intercept only one request");
> +    is(interceptList[0].context, "fetch", "should intercept fetch");
> +    is(interceptList[0].resource, "foo.txt", "should intercept fetch");
> +  }).catch(function(e) {
> +    ok(false, "unexpected promise rejection " + e);

You shouldn't use is and ok like this, since they won't report their results to the main test page.  Instead please do something like:

var is = parent.is;
var ok = parent.ok;

Comment 16

[Ben Kelly [:bkelly, not reviewing]]

(In reply to :Ehsan Akhgari (Away 2/10-2/19) from comment #15)
> Comment on attachment 8717097 [details] [diff] [review]
> P2 Test that service workers do not intercept plugin network requests.
> r=ehsan
> 
> Review of attachment 8717097 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/workers/test/serviceworkers/fetch/plugin/plugins.html
> @@ +1,2 @@
> >  <!DOCTYPE html>
> > +<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
> 
> Please remove this (see below.)
> 
> You shouldn't use is and ok like this, since they won't report their results
> to the main test page.  Instead please do something like:
> 
> var is = parent.is;
> var ok = parent.ok;

I can't since the plugin-utils.js code expects SimpleTest to be loaded.  Also, I verified this fails and reports errors correctly.  I get TEXT-UNEXPECTED-FAIL in the ./mach mochitest output.

Comment 17

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8716984 [details] [diff] [review]
P1 Make plugin network requests bypass service worker interception. r=ehsan

Requesting permission to land P1+P2 on central/aurora/beta/release.  Its not 100% clear to me if this is chemspill worthy, but I'd like it to be considered.

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

An attacker can construct an evil.com site that embeds some malicious flash.  Normally flash performs a preflight request to crossdomain.xml on 3rd party origins, but service worker can be used to intercept and change this data.  This would then allow the malicious flash to access potentially private data on other origins.

Its unclear if flash would get the users cookies in this case or not.

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

The fix and the tests reference intercepting plugins, but not the crossdomain.xml file.  Someone reading the patches would need to figure that part out themselves.

Which older supported branches are affected by this flaw?

FF44+

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

Service Workers.

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

All we need to do is add the LOAD_BYPASS_SERVICE_WORKER flag on these channels.  It should uplift very easily.

How likely is this patch to cause regressions; how much testing does it need?

Minimal since it only adds the LOAD_BYPASS_SERVICE_WORKER flag.  It should only affect service workers.

Comment 18

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Test that service workers do not intercept plugin network requests. r=ehsan

See comment 17.

Comment 19

[Ben Kelly [:bkelly, not reviewing]]

I don't see anything in the plugin code that would set LOAD_ANONYMOUS.  It probably does send credentials.

Comment 20

[Daniel Veditz [:dveditz]]

(In reply to Ben Kelly [:bkelly] from comment #19)
> I don't see anything in the plugin code that would set LOAD_ANONYMOUS.  It
> probably does send credentials.

Some plugins use their own networking stack and manage their own credentials, but most use NPAPI to ask the browser to do so on their behalf and expect credentials (cookies, http auth) to work as well as proxy settings. That's how you're catching the crossdomain.xml request, and other sub-requests would be the same.

Comment 21

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

sec-approval+ as well as other approvals for P1.

I'm not approving the P2 test as I don't want to zero day ourselves with it. We don't normally check in tests for bad security bugs until *after* we've shipped a release with the fix to avoid this problem.

Comment 22

[Ben Kelly [:bkelly, not reviewing]]

Note, we intend to disable service workers on 45esr.  So I think it will be unaffected.

Comment 23

[Ben Kelly [:bkelly, not reviewing]]

There are a couple of other service worker bugs I'd like to have ride along if we can.  Bug 1243453 and bug 1244122.

Comment 24

[Sylvestre Ledru [:Sylvestre]]

> Thanks for confirming it's an e10s-only issue.
Andrew, are you sure about this?

Comment 25

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8716984 [details] [diff] [review]
P1 Make plugin network requests bypass service worker interception. r=ehsan

Release management is not yet ready to manage the chemspill. Reverting the approval until we are.

Comment 26

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #24)
> > Thanks for confirming it's an e10s-only issue.
> Andrew, are you sure about this?

Andrew is on PTO, answering for him.

The original issue about flash video breaking with service workers only happens on non-e10s.  That is not really material, though.

The bigger issue is that the service worker intercepts these requests at all.  And that does happen on both e10s and non-e10s.  This part is the security problem.

Comment 27

[Sylvestre Ledru [:Sylvestre]]

OK, thanks.
I guess this also impacts fennec, right?

Comment 28

[chris hofmann]

sent this to release-drivers but now adding here too for followup investigation.

is this bit like the issue warned of in https://code.google.com/p/chromium/issues/detail?id=413094 back in 2014 and opened to the public in Jan 2015?

dveditz IRC comment was:  The 2nd paragraph is this bug. Don't know if we suffer from the first (maybe?)

also the test url is public increasing risk of discovery.

Comment 29

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Sylvestre Ledru [:sylvestre] from comment #27)
> I guess this also impacts fennec, right?

:snorp tells me fennec runs flash.  So yes, its vulnerable.

(In reply to chris hofmann from comment #28)
> is this bit like the issue warned of in
> https://code.google.com/p/chromium/issues/detail?id=413094 back in 2014 and
> opened to the public in Jan 2015?

Yes, I think its the same issue.

Comment 30

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Ben Kelly [:bkelly] from comment #29)
> (In reply to chris hofmann from comment #28)
> > is this bit like the issue warned of in
> > https://code.google.com/p/chromium/issues/detail?id=413094 back in 2014 and
> > opened to the public in Jan 2015?
> 
> Yes, I think its the same issue.

Jake offered to close the chromium issue again for us.  Let me know if we think that's a good idea.  Seems like some risk of drawing attention to it, though.

Comment 31

[chris hofmann]

If we get the chrome bug closed off we might want to record a few of the details here so people know whats written down there.

here is the summary of the chrome bug.  is it worth grabbing and trying the test case in the chrome bug too?

VULNERABILITY DETAILS
A malicious site can request a non-existent Flash file from a target site. The malicious site's Service Worker onfetch handler (note: requires Experimental Web Platform Features, so I'm marking this Impact-None) intercepts the request and provides a Flash file that appears (to Flash) to originate from the target site. The Flash file can act with the user's cookies on target site.

The onfetch handler can also intercept the request for the target site's crossdomain.xml file and spoof a liberal crossdomain policy, letting the Flash file disclose content on the target site to the malicious site, etc.

VERSION
Chrome Version: all? onfetch has been implemented behind a flag for a while. Note that onfetch is behind Experimental Web Platfor Features flag.
Operating System: all

REPRODUCTION CASE
1. Extract attached ZIP file and run a serve the content of the rosettasw folder at root.
2. Run Chrome with Experimental Web Platform Features enabled.
3. Sign into Facebook.
4. Open http://localhost:8080/ (or whereever you're serving it. Note: Service Workers require a secure origin. Recommend localhost.)
5. Hit reload. Your name on the target site is disclosed to localhost.

For investigation, this logs to the console in the page (make sure "Disable cache" is NOT checked in the Network tab of Inspector) and the Service Worker (use chrome://inspect to inspect the Service Worker.)

---

Once we announce our fix it will probably pique interest in the security of service workers.  Was there one last good security review before we release it in 44 to check for other security related things that turned up in Chrome that researchers might go checking for?

other things like http://sirdarckcat.blogspot.com/2015/10/range-responses-mix-match-leak.html ?

the chrome report, and sirdarkcat's research turned up with a few quick and easy google searches like "service worker exploit";  other more clever searches might turn up more.

Comment 32

[April King [:April]]

I don't know if this bug affects Silverlight as well, but it is probably good to be aware that it has its own version of crossdomain.xml in clientaccesspolicy.xml:

https://msdn.microsoft.com/library/cc197955%28v=vs.95%29.aspx

Comment 33

[Ben Kelly [:bkelly, not reviewing]]

(In reply to April King from comment #32)
> I don't know if this bug affects Silverlight as well, but it is probably
> good to be aware that it has its own version of crossdomain.xml in
> clientaccesspolicy.xml:

The patch in this bug disables service worker interception for all NPAPI generated network requests.  So I believe it would cover silverlight as well.

Comment 34

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8716984 [details] [diff] [review]
P1 Make plugin network requests bypass service worker interception. r=ehsan

Let's take it now for 44.0.2 and 45.0beta5. We will start the build once treeherder is green.

Comment 35

[chris hofmann]

re: comment 17:

> Its unclear if flash would get the users cookies in this case or not.

Seems that's what the first part of the chrome bug is about.  does the the chrome test case help to confirm if that is possible, or help to build a test case around confirming cookie handling?

If I'm reading the bug and the fix right, once the plugin interception happens we should be covered against any cookie mishandling too.  Is that correct?

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/20c93a4cbb1e
https://hg.mozilla.org/releases/mozilla-aurora/rev/3d5173aca497
https://hg.mozilla.org/releases/mozilla-beta/rev/645a5f29ab61

This needs a bit of rebasing for release.

Comment 37

[Ben Kelly [:bkelly, not reviewing]]

Build running.  Should be able to verify the rebase in about 30 minutes.

Comment 38

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Rebased P1 for release branch

I rebased the patch to release and verified it fixes the original issue noted in this bug.

I also downloaded the exploit PoC from the chromium bug.  I could *not* reproduce the exploit in current release FF44.  Since we block interception of the top level .swf file it could not load its Exfiltrator.swf under the facebook origin.

So thats good news.

We do still allow the crossdomain.xml file to be intercepted, though.  That's not enough for this particular PoC, but it still seems potentially bad to allow this.

With the P1 patch we do not intercept the crossdomain.xml either.

David, do you still think this is chemspill worthy if the currently public PoC exploit does not work on our currently released FF44?

Comment 39

[Daniel Veditz [:dveditz]]

(In reply to Ben Kelly [:bkelly] from comment #22)
> Note, we intend to disable service workers on 45esr.

Why? if it's not stable enough for ESR it really wasn't done/stable/safe enough to have released the version before that ESR -- should have stayed in beta.

Comment 40

[Daniel Veditz [:dveditz]]

(In reply to Ben Kelly [:bkelly] from comment #38)
> [Daniel], do you still think this is chemspill worthy if the currently public
> PoC exploit does not work on our currently released FF44?

Yes. We were talking about chemspilling for this (because it's a Universal XSS and seems reasonably easy to re-discover) before we found the PoC in the public Chrome bug. Their PoC relies on what would be two separate bugs in our code, and apparently we only had one. But the one we have is definitely bad enough.

Comment 41

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Daniel Veditz [:dveditz] from comment #39)
> Why? if it's not stable enough for ESR it really wasn't done/stable/safe
> enough to have released the version before that ESR -- should have stayed in
> beta.

We publicly discussed this approach in Dec 2015:

  https://groups.google.com/forum/#!topic/mozilla.dev.platform/yuNHtDhl3lY

No one expressed the position or policy that you are suggesting.  The e10s team was talking about doing the same thing if they shipped in 45.  I have been told other features have shipped this way as well.

Regardless of service workers, I personally don't agree that we should only ship things in normal train releases that we require to be stable in API/design over the next 9 months.

Anyway, if you feel this sort of policy change should occur, I suggest sending email to the lists so it can be discussed.

Comment 42

[Ryan VanderMeulen [:RyanVM]]

Pushed to m-r based on comment 40.
https://hg.mozilla.org/releases/mozilla-release/rev/952e2822bae8

Comment 43

[Andrew McCreight [:mccr8]]

Could somebody who understands the issue please update the summary to be more specific to the security issue? Thanks.

Comment 44

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/20c93a4cbb1e

Comment 45

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced this issue on Mac OS X 10.9.5 using latest Nightly (2015-02-09) with the STR available in the description.

Confirming the fix on Firefox 44.0.2 (build ID 20160209150140). Tested on Windows 7 64bit, Mac OS X 10.9.5 and Ubuntu 12.04 x86.

Comment 46

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8717114 [details] [diff] [review]
P2 Test that service workers do not intercept plugin network requests. r=ehsan

I would prefer to let this ride the beta45 -> release45 train. Thanks!

Comment 47

[Ben Kelly [:bkelly, not reviewing]]

Comment on attachment 8717114 [details] [diff] [review]
P2 Test that service workers do not intercept plugin network requests. r=ehsan

Since we're waiting to land the tests I'm just going to not uplift them.  I'll land in nightly when appropriate.

Comment 48

[u421692]

Unable to reproduce this on Firefox for Android following the STR from the description, since the page displays HTML5 video even with flash player installed.
Can you please tell me how can I register service worker on another page?

Comment 49

[Ben Kelly [:bkelly, not reviewing]]

(In reply to Mihai Pop from comment #48)
> Unable to reproduce this on Firefox for Android following the STR from the
> description, since the page displays HTML5 video even with flash player
> installed.
> Can you please tell me how can I register service worker on another page?

Well, you would have to build a new demo site that serves a flash player, but explicitly avoids html5 video.  You can't add a service worker to an existing site since the service worker must be same origin.

If you are building your own demo site, you just do:

  navigator.serviceWorker.register('empty.js');

And then reload the page.