957980 - [META] expand CSP enforcement capabilities to include XUL features

Status: NEW

(Core :: DOM: Security, enhancement, P3)

Description

[Frederik Braun [:freddy]]

If we want CSP to effectively protect internal pages (like about:foo) from XSS, we need to make sure that XUL-specific scripting features are disallowed.

One example would be XUL Commands as this allows to trigger certain browser actions without scripting. Browser:Open with javascript URLs looks like a potential issue. See https://developer.mozilla.org/en-US/docs/XUL/List_of_commands

Another example might be XBL.

Comment 1

[Frederik Braun [:freddy]]

This bug is actually just a smart guess. 
Garett, do you know the code well enough to help us elaborate on this?

Comment 2

[Garrett Robinson [:grobinson]]

I'm not familiar with the XUL-specific code, although I agree that your Browser:Open example seems troublesome. Generally, you would just need to put in hooks to check for CSP wherever a CSP-controlled resource might be loaded or executed. Example: http://dxr.mozilla.org/mozilla-central/source/content/base/src/nsScriptLoader.cpp#708 (CSPAllowsInlineScript in nsScriptLoader.cpp, in case it rots).

Comment 3

[Frederik Braun [:freddy]]

Attachment: test case for inline script in XUL pages

I'm afraid that XUL pages aren't suspect to CSP at all...and this applies to most about: pages we want to apply CSP to.

STR (in a clean profile:
1: Install Remote XUL https://addons.mozilla.org/en-US/firefox/addon/remote-xul-manager/
2: Tools, Web Developer, Remote XUL, allow "localhost" (it won't show in the list however?)
3: Place attached test case somewhere web accessible on localhost
4: Navigate to test case (i.e., sample.xul.php)
5: Click the button
5: Observe, in the developer console, that three console.log() calls have been executed:
One through <script> elements, one through <html:script> and one through oncommand.

I assuming using the Remote XUl add-on didn't somehow allow for a CSP bypass.

Comment 4

[Frederik Braun [:freddy]]

(In reply to Frederik Braun [:freddyb] from comment #3)
> I'm afraid that XUL pages aren't suspect to CSP at all...and this applies to
> most about: pages we want to apply CSP to.

s/suspect/subject/
:)

Comment 5

[Garrett Robinson [:grobinson]]

(In reply to Frederik Braun [:freddyb] from comment #3)
> Created attachment 8367852 [details]
> test case for inline script in XUL pages
> 
> I'm afraid that XUL pages aren't suspect to CSP at all...and this applies to
> most about: pages we want to apply CSP to.

That's unfortunate. I had hoped that non-XUL-specific features would share a common implmentation, but this test case appears to disprove that.

There are at least some locations in the XUL code where others have noted would need hooks for CSP. For example, content/xul/content/src/nsXULElement.cpp:2324, to check style="" attributes on XUL elements.

Comment 6

[Sid Stamm [:geekboy or :sstamm]]

To be clear, seems to me this isn't a problem with the content policy, it's a problem with the CSP hooks, right?  It's not so much that loads are skipping nsIContentPolicy::ShouldLoad calls, but rather that CSP isn't blocking inline scripts and event handling attributes and such in XUL documents.  That should be somewhat simple to fix, IMO.

Right now most XUL pages skip through CSP because we don't check chrome:// resources.  We could easily change that... but it would break a bunch of gecko if we didn't allow inline XUL scripts.

freddyb: I assigned this bug to you since you filed it and we need to find an owner if we want to fix it, can you help clarify the funtamental problem and then we can try to find a new owner?

Comment 7

[Frederik Braun [:freddy]]

Going through old bugs, I realized I have yet to respond to this.
Leaving a needinfo for myself to make sure I'll get back to it throughout the week.

Comment 8

[Frederik Braun [:freddy]]

XUL offers certain features that are not covered by our CSP implementation. This includes, amongst others, non-HTML tags and attributes to load styles, images or execute scripts.

Some examples:
* https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XUL/Attribute/oncommand (similar to inline JavaScript)
* https://developer.mozilla.org/en-US/docs/XBL (XUL Bindings allow execution of JavaScript through styles)
* the <browser> element creates a nested browsing context similar to iframes, that may not be subject to the respective CSP directives

Comment 9

[Frederik Braun [:freddy]]

(In reply to Sid Stamm [:geekboy or :sstamm] from comment #6)
> freddyb: I assigned this bug to you since you filed it and we need to find
> an owner if we want to fix it, can you help clarify the funtamental problem
> and then we can try to find a new owner?

Oh, this bug sat in my list long enough. It still needs a real owner.

Comment 10

[Christoph Kerschbaumer [:ckerschb}]

Freddy, besides that this bug would need an owner. Are we still planning on doing this? More general, what's the status of this? Do you happen to know by any chance?

Comment 11

[Frederik Braun [:freddy]]

I think this circles back to the question whether anybody's interested in doing bug 923902.
Given how old this bug here is, I suppose not many?
In the long run (especially post-sandbox), it would be an obvious security win to protect chrome-privileged pages with a CSP.
But it might be more interesting to require this for the HTML pages and leave the XUL pages as they are while we transition away from them?

Though I do wonder if we can prevent a privileged HTML page from using XUL features (in iframes, popups, data URIs, etc.) at all.

Comment 12

[:Gijs (away/busy until Mar 10)]

Christoph, now that XBL is dead, do you know off-hand if other XUL oddities (like inline command event handlers) are already subject to CSP? If you don't know off-hand, don't worry about investigating, I can probably find a few moments to do it...

Comment 13

[Christoph Kerschbaumer [:ckerschb}]

(In reply to :Gijs (he/him) from comment #12)

Christoph, now that XBL is dead, do you know off-hand if other XUL oddities (like inline command event handlers) are already subject to CSP? If you don't know off-hand, don't worry about investigating, I can probably find a few moments to do it...

Hey Gijs, I am not 100% sure but I guess that comment around oncommand handlers being subject to CSP within contentAreaDownloadsView.xhtml indicates that they are.

Comment 14

[:Gijs (away/busy until Mar 10)]

(In reply to Christoph Kerschbaumer [:ckerschb] from comment #13)

(In reply to :Gijs (he/him) from comment #12)

Christoph, now that XBL is dead, do you know off-hand if other XUL oddities (like inline command event handlers) are already subject to CSP? If you don't know off-hand, don't worry about investigating, I can probably find a few moments to do it...

Hey Gijs, I am not 100% sure but I guess that comment around oncommand handlers being subject to CSP within contentAreaDownloadsView.xhtml indicates that they are.

Oh right. Hm, in that case, maybe we're done here? Freddy?

Comment 15

[Frederik Braun [:freddy]]

I'm entirely too new here to know anything about XUL, but a cursory look at https://developer.mozilla.org/en-US/docs/Archive/Mozilla/XUL/XUL_Reference tells me, this is good.
I've scanned for other elements that look "active" and stumbled upon <observe> and <broadcast> and <overlay> etc, but I don't think anything comes close to scripting. Maybe CSP support for XUL is OK?

NB: I also want to note that we're already sanitizing the badness out of everything that could look like DOM XSS in nsContentUtils::ParseHTML & ParseXML.
The protection we are missing without a CSP is for pages that are not generated by JavaScript during page-load/execution, but generated differently like about:cache, which is created from C++ code (but does have a CSP)