[META] expand CSP enforcement capabilities to include XUL features like XBL

NEW
Unassigned

Status

()

enhancement
P3
normal
6 years ago
3 months ago

People

(Reporter: freddyb, Unassigned)

Tracking

(Blocks 2 bugs, {meta})

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [domsecurity-meta])

Attachments

(1 attachment)

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.
This bug is actually just a smart guess. 
Garett, do you know the code well enough to help us elaborate on this?
Flags: needinfo?(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).
Flags: needinfo?(grobinson)
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.
(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/
:)
(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.
Component: Security → DOM: Security
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
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?
Summary: CSP does not block XUL Features → expand CSP enforcement capabilities to include XUL documents
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.
Flags: needinfo?(fbraun)
Flags: needinfo?(fbraun)
Summary: expand CSP enforcement capabilities to include XUL documents → [META] expand CSP enforcement capabilities to include XUL features like XBL
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
(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.
Assignee: fbraun → nobody
Status: ASSIGNED → NEW
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?
Flags: needinfo?(fbraun)
Whiteboard: [domsecurity-backlog]
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.
Flags: needinfo?(fbraun)
Whiteboard: [domsecurity-backlog] → [domsecurity-meta]
Assignee: nobody → fbraun
Status: NEW → ASSIGNED
Assignee: fbraun → nobody
Status: ASSIGNED → NEW
Depends on: 965637
Priority: -- → P2
Type: defect → enhancement
Priority: P2 → P3
You need to log in before you can comment on or make changes to this bug.