1346273 - about:healthreport embedded page (fhr.cdn.mozilla.net) has missing CSP headers

Status: RESOLVED WONTFIX

(Infrastructure & Operations Graveyard :: WebOps: Other, task)

Description

[Abdulrahman Alqabandi]

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36

Steps to reproduce:

If you navigate to 'about:healthreport' you will be within a chrome privileged page which contains an iframe pointing to 'https://fhr.cdn.mozilla.net/en-US/v4/'

This is not unusual for about: pages, for example 'about:accounts' is similar except the iframe points to 'https://accounts.firefox.com/signup'

The problem is that 'fhr.cdn.mozilla.net' does not contain any CSP headers which means that if that domain were to contain malicious JS (from add-on redirection, or even if domain is hijacked) this malicious JS will execute.

The embedded page can have a reference to the chrome window using window.top

Steps:

1. Go to about:healthreport 
2. Open WebConsole and execute: this['remote-report'].contentWindow.location='data:text/html,<script>alert(0)</script>'
3. Alert is popped
4. Go to about:accounts
5. Open WebConsole and execute: remote.contentWindow.location='data:text/html,<script>alert(0)</script>'

blocked via csp:
[JavaScript Error: "Content Security Policy: The page’s settings blocked the loading of a resource at self (“script-src https://accounts.firefox.com https://accounts.cdn.mozilla.net”). Source: alert(0)." {file: "https://accounts.firefox.com/signup?service=sync&context=fx_desktop_v3" line: 1 column: 0 source: "alert(0)"}]



Actual results:

JS executed within embedded page


Expected results:

A CSP header specifying script-src

Comment 1

[:Gijs (he/him)]

Do we even still need/use about:healthreport? I thought FHR was dead, replaced by UT, and so we can maybe just unship this?

Comment 2

[Alessio Placitelli [:Dexter]]

Thanks for reporting this.

(In reply to :Gijs from comment #1)
> Do we even still need/use about:healthreport? I thought FHR was dead,
> replaced by UT, and so we can maybe just unship this?

We do use about:healthreport, which is now powered by UT data.

Comment 3

[Georg Fritzsche [:gfritzsche]]

about:healthreport is still in use for a few reasons, we can't directly un-ship this.

Comment 4

[Georg Fritzsche [:gfritzsche]]

fhr.cdn.mozilla.net is managed by ops AFAICT.
Documentation for the service is here:
https://mana.mozilla.org/wiki/pages/viewpage.action?pageId=33101035

Comment 5

[Julien Vehent]

Note that about:* pages ignore CSP, so while it may be good practice to enable it, it won't help protect about:healthreport should the FHR start serving fraudulent scripts.

Comment 6

[:Gijs (he/him)]

(In reply to Julien Vehent [:ulfr] from comment #5)
> Note that about:* pages ignore CSP

Can you clarify what you mean by this? I'm a bit surprised at this assertion, certainly if you also mean that iframes with different URLs on such pages (which is the case here) also ignore CSP...

Comment 7

[Julien Vehent]

:Gijs - we did through this with :freddyb and the proper statement here is

> When framing a site into a local resource (about:, chrome:, data:, blob:, resource:, moz-icon:, but not file:), the CSP is ignored except for frame-ancestors directives that get applied anyway.

Comment 8

[:Gijs (he/him)]

(In reply to Julien Vehent [:ulfr] from comment #7)
> :Gijs - we did through this with :freddyb and the proper statement here is
> 
> > When framing a site into a local resource (about:, chrome:, data:, blob:, resource:, moz-icon:, but not file:), the CSP is ignored except for frame-ancestors directives that get applied anyway.

Then there should probably be a followup for how we mitigate risk for both about:healthreport and about:accounts. Freddy?

Comment 9

[Frederik Braun [:freddy]]

about:healthreport can frame things that have an X-Frame-Option header of DENY.
I tested this by executing the following code in DevTools, while on about:healthreport.
> document.body.innerHTML = `<iframe src="https://freddyb.dubhe.uberspace.de/eimer/xfo.php"></iframe>`
Does that help you?

Comment 10

[Abdulrahman Alqabandi]

I'm confused, wasn't step 5 in my original report indicative of CSP being applied in a chrome window? What am I missing?
Also I would expect chrome to have access to cross origin windows since we can do much more (like pop calc) but I would like to see more checks on integrity of the data coming from these framed sites. Whether these checks are CSP or perhaps utilizing a white-list.

Comment 11

[Abdulrahman Alqabandi]

Also, does 'about:healthreport' need to be full on chrome privileged? Why cant it be like about:home and friends which dont have top level privilege?

Comment 12

[Frederik Braun [:freddy]]

NB: about:accounts uses an iframe with mozframetype="content"
>     <iframe mozframetype="content" id="remote" />

FHR does
>     <iframe id="remote-report"/>

Maybe mozframetype can be of help here?

Comment 13

[Abdulrahman Alqabandi]

Bug 769771 seems related

Comment 14

[Frederik Braun [:freddy]]

(In reply to Julien Vehent [:ulfr] from comment #7)
> :Gijs - we did through this with :freddyb and the proper statement here is
> 
> > When framing a site into a local resource (about:, chrome:, data:, blob:, resource:, moz-icon:, but not file:), the CSP is ignored except for frame-ancestors directives that get applied anyway.


I think this is slightly misrepresenting how things work, let me re-iterate:

* In general CSP declares what loads may and may not happen (for scripts, images, etc.) but also which sites are allowd to be parent frames.
Those loads look strictly at the whitelist in the policy, but it _does_ know exceptions for add-ons and browser code, because it has higher privileges (and priorities) than  website authors

The fact that frame-ancestors *does* block some chrome pages as parent pages (but not all) is a bug in the frame-ancestors implementation. See bug 1346774.



For the meantime, you might want to use a CSP to restrict everything _but_ frame ancestors first and then find a way around that. I proposed to options: 
a) Figure out the difference between an <iframe> (currently used in FHR) and an <iframe mozframetype="content" (currently used within about:accounts) 
b) X-Frame-Options: DENY in FHR, the X-Frame-Options header does not appear to have the same bug.

Comment 15

[:Atoll]

(In reply to Frederik Braun [:freddyb] from comment #14)
> b) X-Frame-Options: DENY in FHR, the X-Frame-Options header does not appear
> to have the same bug.

To confirm, if we advertise XFO:DENY on responses from fhr.data.mozilla.com, that's sufficient to workaround the issues described in this bug?

Comment 16

[:Gijs (he/him)]

(In reply to Richard Soderberg [:atoll] from comment #15)
> (In reply to Frederik Braun [:freddyb] from comment #14)
> > b) X-Frame-Options: DENY in FHR, the X-Frame-Options header does not appear
> > to have the same bug.
> 
> To confirm, if we advertise XFO:DENY on responses from fhr.data.mozilla.com,
> that's sufficient to workaround the issues described in this bug?

--> Freddy?

(I think you need more general CSP like whatever accounts.firefox.com is doing, but I defer to Freddy.)

Comment 17

[:Atoll]

Okay. So, basically, WebOps has no actionable response to this bug at this time. We're happy to support y'all making changes here to meet whatever needs you're trying to meet, but the above discussion offers no certainty about any changes we should make :)

Comment 18

[Frederik Braun [:freddy]]

Yes. If you want to protect against evil websites putting fhr.data.mozilla.com in a frame, use X-Frame-Options: DENY.
I only tested on Nightly and I know the code hasn't changed much, but I would strongly I advise testing on all branches.

Comment 19

[Frederik Braun [:freddy]]

(In reply to Julien Vehent [:ulfr] from comment #5)
> Note that about:* pages ignore CSP, so while it may be good practice to
> enable it, it won't help protect about:healthreport should the FHR start
> serving fraudulent scripts.

Well, about:healthreport *would* benefit if fhr.data.mozilla.com was getting a strong CSP. And I think we should start with that. (The XFO/frame-ancestor stuff is not that important for this bug and a good low/medium priority follow up)

But fhr.data.mozilla.com uses inline scripts so we'd need to start with that.
Unfortunately, this is not something I can drive.

April or Julien: Can you help driving this?
I'd be happy to fill you in on IRC, if you have any more questions.

Comment 20

[Frederik Braun [:freddy]]

There was a confusion as to which site we are talking about.
fhr.data.mozilla.com seems to be a legacy thing.
The hostname in question is fhr.cdn.mozilla.net

Comment 21

[Julien Vehent]

Jason: could you help route this to the right people? Two things are needed to close this bug:

1. refactor the HTML/CSP/JS served from fhr.cdn.mozilla.net to work with a CSP (it currently uses inline scripts)

2. serve the CSP header through the CDN

Comment 22

[April King [:April]]

I don't know exactly where on the CDN it pulls that page from (all I see is the Redhat Apache splash page), but I'd be happy to assist with getting a proper CSP policy going there.

Comment 23

[:Atoll]

It's hosted through it-cdn. Y'all are welcome to send this back to WebOps: Other to have the team look into it, but please clearly state the desired CSP policy you would like to see deployed and/or request an XFO:DENY header.

Comment 24

[:Atoll]

(the desired CSP policy, in valid CSP syntax, just to remove any uncertainty about what's being asked there)

Comment 25

[:Atoll]

Presumably:

modules/webapp/files/genericrhel6/etc-httpd/domains/fhr.cdn.mozilla.net.conf:    ServerAlias fhr-origin.cdn.mozilla.net

Comment 26

[Jason Thomas [:jason]]

(In reply to Julien Vehent [:ulfr] from comment #21)
> Jason: could you help route this to the right people? Two things are needed
> to close this bug:
> 
> 1. refactor the HTML/CSP/JS served from fhr.cdn.mozilla.net to work with a
> CSP (it currently uses inline scripts)

There are several people that could potentially work on making modifications to https://github.com/mozilla/fhr-jelly. I've cc'd :kparlante for input.

> 2. serve the CSP header through the CDN

Moving to to Webops:Other as per comment 23.

Comment 27

[:Atoll]

Okay, so, this unowned bug has transferred ownership too many times, and a lengthy discussion has ensued, producing no concrete requests of WebOps that we can take action on.

I'm going to close this WebOps bug as INCOMPLETE, but please reopen it if/when y'all determine the precise CSP and/or XFO header changes you wish us to make.

Comment 28

[Georg Fritzsche [:gfritzsche]]

(In reply to Richard Soderberg [:atoll] from comment #27)
> Okay, so, this unowned bug has transferred ownership too many times, and a
> lengthy discussion has ensued, producing no concrete requests of WebOps that
> we can take action on.

I don't think that is a good reason to close a valid bug. We should instead figure out what needs to be done and by who.

(In reply to Julien Vehent [:ulfr] from comment #21)
> 1. refactor the HTML/CSP/JS served from fhr.cdn.mozilla.net to work with a
> CSP (it currently uses inline scripts)
> 
> 2. serve the CSP header through the CDN

Julien, can you specify what exactly is needed for (1)?
While i'm missing the knowledge, i can do changes on fhr-jelly.

Can you also be more specific of whats needed for (2)?
Then i can probably file a follow-up bug for that part.

Comment 29

[Julien Vehent]

Looking at https://fhr.cdn.mozilla.net/en-US/v4/, the main problem is the activation widget at the bottom would require unsafe-inline in the CSP. We can use a nonce because this is a static asset (nonce need to change with every request to be safe), so we either need to set a CSP with unsafe-inline (which is bad), or move that inline script to a separate file.

Once that's done, we should be able to serve a CSP header via the CDN. I think April offered her services in writing a good policy.

Comment 30

[:Atoll]

The first step in shipping CSP is to ship CSP for what you have today, no matter how unpalatable it is. There's no CSP policy today, so including "unsafe-inline" is still more secure than "unsafe-everything" as it is right now.

1) April, can you define a CSP policy that correctly describes the site as it is today, including "unsafe-inline"?

2) Georg, can you file the bug for moving the activation widget inline script to a separate file?

Comment 31

[April King [:April]]

So this is what Laboratory gave me:

>  default-src 'none'; connect-src 'self'; font-src 'self'; img-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline'

But I suspected it was missing things because of how it tries to do things outside its scope.

I tried to do this a better way (via a mitm proxy), but for some reason despite setting 'unsafe-inline' it still is blocking inline script for some unknown reason.

This is closer, from what I could get with Chrome:

> default-src 'none'; connect-src 'self'; font-src 'self'; img-src 'self'; script-src 'self' 'sha256-c38ba39cea630681f6bdc6acc7eade251530622bc6f10dda7f1fd77af189a1df' 'sha256-PVLtXptN3UfLqK7vhXN3XAg9D9fdlzxLk/khrFs87Q4='; style-src 'self' 'unsafe-inline'

But Firefox's CSP dev tools (which I have to use here) are so unhelpful that I can't actually proceed any further. It's complaining about things like:

> Content Security Policy: The page’s settings blocked the loading of a resource at self (“script-src https://fhr.cdn.mozilla.net 'sha256-c38ba39cea630681f6bdc6acc7eade251530622bc6f10dda7f1fd77af189a1df' 'sha256-PVLtXptN3UfLqK7vhXN3XAg9D9fdlzxLk/khrFs87Q4='”). Source: onload attribute on UNKNOWN element.

With no line number and no element.  The only onload I can find is init(), which should be covered by the shasum above. It's as if Firefox is not able to use hashes for CSP in this context, but I'm not 100% certain.

Overall, I would say this is one of the more questionable design decisions that I have seen. We have a privileged page that is loading an external iframe, which then using its privileged access to call into Components.utils for the sole purpose of... generating a chart and providing some numbers.

I get _why_ it's an external resource, as it allows changes to be made without riding the trains but overall it seems like it would be better to either:

a) move this page into Firefox, or
b) have about:healthreport take all the random data that it needs, stuff it into GET parameters, put those into the iframe's src, and sandbox the heck out of the iframe.

I'm going to submit a patch to FHR to fix this particular piece of inline script so that I don't need to use a shasum for it.

Comment 32

[April King [:April]]

PR submitted:

https://github.com/mozilla/fhr-jelly/pull/181

Comment 33

[Georg Fritzsche [:gfritzsche]]

Thanks for untangling this.

(In reply to April King [:April] from comment #31)
> I get _why_ it's an external resource, as it allows changes to be made
> without riding the trains but overall it seems like it would be better to
> either:
> 
> a) move this page into Firefox, or
> b) have about:healthreport take all the random data that it needs, stuff it
> into GET parameters, put those into the iframe's src, and sandbox the heck
> out of the iframe.

The design decision behind it was to not have the data leave the client.
That way this page can work even when we the users disables all FHR/Telemetry data upload.
FWIW, i don't think the fast off-train delivery was ever needed here.

There are on-going conversations now again for removing about:healthreport (bug 1352497), so we should probably not put more effort into this.
It is unclear what the time-frame is for this though.

Comment 34

[April King [:April]]

Okay, I've been doing some more poking away at it and now have a better idea of the design. I still think it's pretty weird to do it this way, but I had been operating under the assumption that FHR was granted privilege, which it isn't.

1) about:healthreport loads an FHR in an unprivileged iframe
2) it then does a postMessage to FHR which consumes the data and presents the iframe

If we're looking at CSP, we're looking to address two XSS possibilities:

1) a reflected XSS:
  a) if on FHR directly, it wouldn't receive the message from about:, and so there wouldn't really be anything for it to do
  b) if on about:healthreport, we probably have a lot more serious issues than the data that FHR gets

2) a stored XSS:
  a) if on FHR, any visit to about:healthreport would be able to steal the browser data that FHR reports, but there's nothing stored there so no real way to cause this problem
  b) can't exist in about:healthreport unless we put it there ourselves

Now I do think we should get CSP going on FHR should we want to keep it around, but given my understanding of the architecture (thanks freddyb), I don't really see how an XSS could cause anything but defacement.

freddyb, you mentioned concerns about postMessages. As near as I can tell, the messages only come in one direction: healthreport -> FHR. I would be a lot more concerned about the other way around, but since it's a different origin it *shouldn't* be able to send postMessages to the parent, unless there is somewhere I'm missing in about:healthreport that is accepting messages.

One thing I did notice is that FHR's message receiving doesn't verify that the origin of the message, so presumably another site could frame FHR and possibly cause a piece of code to execute in FHR's context.

Looking at this code (keeping in mind that I am only doing a cursory glance at this here) I don't see it setting anything via .html() or anything via .css() that is dangerous. So I *think* we're probably safe from malicious messages, but nevertheless we should probably try to make CSP work (and verify the origin) should we attempt to keep FHR around.

freddyb or dveditz, does my summary look okay here? Sorry for the rambling, just trying to get a grasp of the scope of the problem from an outside position.

Comment 35

[Frederik Braun [:freddy]]

(In reply to April King [:April] from comment #34)
> freddyb or dveditz, does my summary look okay here? Sorry for the rambling,
> just trying to get a grasp of the scope of the problem from an outside
> position.

Your assessment makes sense. Thank you for digging into this!

Comment 36

[Georg Fritzsche [:gfritzsche]]

(In reply to April King [:April] from comment #34)
> One thing I did notice is that FHR's message receiving doesn't verify that
> the origin of the message, so presumably another site could frame FHR and
> possibly cause a piece of code to execute in FHR's context.

We added an origin check in bug 870870:
https://dxr.mozilla.org/mozilla-central/rev/981da978f1f686ad024fa958c9d27d2f8acc5ad0/browser/base/content/abouthealthreport/abouthealth.js#112

Comment 37

[April King [:April]]

Yeah, about:healthreport has an origin check, but FHR doesn't:

https://github.com/mozilla/fhr-jelly/blob/master/js/data_v4.js#L357

I'm not sure this is an actual problem though -- I could frame FHR myself and possibly inject a message to cause an XSS, but I don't think I would be able to cause much issue other than perhaps spewing cookies on .mozilla.net. FHR doesn't really contain any data itself. And I can't iframe or window.open() about:healthreport, so I can't pass messages to FHR in a way that might percolate up to about:healthreport.

Overall it's a pretty small risk but an origin check (for "null", I believe) might be worth adding simply because it's one of (if not the only) part(s) of FHR that accepts arbitrary input.

Comment 38

[Georg Fritzsche [:gfritzsche]]

(In reply to April King [:April] from comment #37)
> Overall it's a pretty small risk but an origin check (for "null", I believe)
> might be worth adding simply because it's one of (if not the only) part(s)
> of FHR that accepts arbitrary input.

Ah, i see. I filed bug 1372519 on this.

To be sure: to first enable CSP we need bug 1371690 + your PR [1] on the production server?

1: https://github.com/mozilla/fhr-jelly/pull/181

Comment 39

[April King [:April]]

Yes, I believe those are the correct prerequisites to enable a strong CSP on FHR.

Comment 40

[Julien Vehent]

Per Freddy, wontfixing because unimplemented.