Closed
Bug 379644
Opened 18 years ago
Closed 17 years ago
let's kill xbl for untrusted content
Categories
(Core :: XBL, defect, P2)
Core
XBL
Tracking
()
RESOLVED
DUPLICATE
of bug 546857
People
(Reporter: guninski, Assigned: sicking)
References
Details
Attachments
(1 file)
(deleted),
text/xml
|
Details |
xbl is constantly causing troubles and it is not clear if it is manageable with current resources:
fork()ed from Bug 378518 Comment #4
G30rgi 2007-05-03 06:38:13 PDT
what about an option |disallow xbl| from userland?
probably the web will be usable and chrome will be unaffected.
Comment #5 [reply] Boris Zbarsky 2007-05-03 07:45:05 PDT
That's an interesting question. Until recently, this hasn't so much been an
option. At this point we _could_ do it. The only question is how many apps it
would break.
Comment #6 [reply] G30rgi 2007-05-03 08:38:20 PDT
(In reply to comment #5)
> The only question is how many apps it would break.
>
probably not exactly "apps" but "unprivileged web stuff".
not many people cry that "extends" in xbl causes SEGV.
Comment #7 [reply] G30rgi 2007-05-03 08:48:58 PDT
(In reply to comment #6)
> (In reply to comment #5)
> > The only question is how many apps it would break.
> >
>
> "unprivileged web stuff".
>
"disable xbl option" is less than clever.
to put it this way:
for standard compliance reasons and the unlimited power of XBL usage of XBL
from luserland requires universalbrowserwrite privilege.
Comment #8 [reply] Boris Zbarsky 2007-05-03 09:20:40 PDT
Unfortunately, there's no way to request that privilege in declarative content
(e.g. CSS).
And I do mean "apps" as in "intranet apps"...
Yes, I would not have made XBL available to untrusted content up front, but the
cat is mostly out of the bag now. Can we please take discussion about that
aspect to a separate bug and stop adding things to this one?
Comment 1•18 years ago
|
||
Does this mean "Continue to allow content to bind to XBL bindings which are available in chrome, but not arbitrary bindings"?
I don't think we have the luxury of breaking remote XUL.
Comment 2•18 years ago
|
||
So what is currently possible is to determine exactly where the -moz-binding declaration came from. Which gives us three pieces of data:
1) The principal of the source of the -moz-binding declaration (e.g. xul.css,
etc).
2) The binding URI.
3) The principal of the node the binding is being applied to.
At the moment, we allow binding attachment if:
a) The binding URI is a chrome:// URI
or
b) The principal of the -moz-binding declaration source can link to the
binding URI (as represented by CheckLoadURI).
I'd like to know what G30rgi's exact proposal is for changes to this setup.
Comment 3•18 years ago
|
||
Note also that we should consider deployment of:
* http://dean.edwards.name/moz-behaviors/usage/
* http://www.activewidgets.com/ (based on http://www.activewidgets.com/javascript.forum.13633.1/firefox-1-5-and-data.html comment)
I stopped on page 2 of the Google listing.
Updated•18 years ago
|
Blocks: killsbabies
Comment 4•18 years ago
|
||
Does this bug need to be security-sensitive? I don't see anything in this bug that sounds like it needs to be, and non-security people should be part of this discussion...
Comment 5•18 years ago
|
||
I don't think it should, no.
Of course I also think it's in the wrong product...
Updated•18 years ago
|
Assignee: nobody → general
Group: security
Component: Security → XBL
Product: Firefox → Core
QA Contact: firefox → ian
Assignee | ||
Comment 6•18 years ago
|
||
I don't think we can do this. If we allowed binding to chrome bindings (in order to not break marquee and xul) we would still be exposable to all the issues that we currently have.
Reporter | ||
Comment 7•18 years ago
|
||
neither blame nor flame BZ, just feel for him for having to fix the
current state of XBL.
the current state of XBL is known and interesting kludges are starting
to emerge - Bug 378518
i am no xbl expert and the purpose of this bug is to disallow as much
xbl as possible while not breaking chrome and pissing as low
people/companies as possible (unless XBL is your hidden ace card).
afaict this potentially may be done by an option disabling untrusted
bindings or requiring a privilege for untrusted bindings - that is,
untrusted content doing |-moz-binding: P0Wned.com| and similar
equivalent stuff.
can't comment how much developers this will piss off, but the fact that
developers aren't noticing |extends| causes SEGV makes me bet
`negligible' number of developers may be pissed off.
sure this proposal is unpopular, but i support the following:
making *all* users happy is mistake leading to unmanageable
features/bugs/bloat.
imho the goal should be to implement the *minimal functionality that
will make about 80%-90% of all browser users happy* (basically this
seems the opera approach)
https://wiki.ubuntu.com/WinningTheDesktop
BZ's examples show some developers will be pissed off - it is unclear
what percentage of current users will be pissed off.
feel free to resolve this bug as you wish.
Reporter | ||
Comment 8•18 years ago
|
||
(In reply to comment #3)
>
> * http://www.activewidgets.com/ (based on
> http://www.activewidgets.com/javascript.forum.13633.1/firefox-1-5-and-data.html
> comment)
>
[ActiveWidgets]$ grep -rniI binding *
[ActiveWidgets]$ grep -rniI moz-binding *
[ActiveWidgets]$
Reporter | ||
Comment 9•18 years ago
|
||
tried to measure the popularity of "-moz-binding" this way:
+"moz-binding" +url -chrome -resource -mBinding -NS_LITERAL_STRING -xss -nsCAutoString
http://www.google.com/codesearch?hl=en&q=+%2B%22moz-binding%22+%2Burl+-chrome+-resource+-mBinding+-NS_LITERAL_STRING+-xss+-nsCAutoString&start=90&sa=N
+"-moz-binding" -chrome -resource -xss -vulnerability -marquee
http://www.google.com/search?q=%2B%22-moz-binding%22+-chrome+-resource+-xss+-vulnerability+-marquee&hl=en&start=30&sa=N
Assignee | ||
Comment 10•18 years ago
|
||
You also need to search for uses of XUL since this would break remote XUL.
Comment 11•18 years ago
|
||
And this would break at least custom controls in XForms, if not whole XForms.
Comment 12•18 years ago
|
||
Which "this" do comment 10 and comment 11 refer to? That is, what modifications are people thinking about to the model in comment 2?
And as a second question, what modifications would actually help the security situation?
Comment 13•18 years ago
|
||
(In reply to comment #11)
> And this would break at least custom controls in XForms
if -moz-binding: url(http://url/to/some/content/xbl) is disabled in normal content
> if not whole XForms.
if -moz-binding: url(chrome://something) is disabled in normal content
(this is the same as remote XUL). The .css file used for binding is in this case also in chrome://
Comment 14•18 years ago
|
||
One possibility is:
* Disallow content pages from introducing new bindings, but allow chrome and user style sheets to introduce bindings to content pages.
* Disallow content pages from using getAnonymousNodes, so they can't exploit insertion-point bugs.
Comment 15•18 years ago
|
||
That second bullet point would mean that a binding attached to an untrusted node also couldn't call getAnonymousNodes (since it's running with the permissions of the node it's attached to). Would that cause problems?
Assignee | ||
Comment 16•18 years ago
|
||
Disabling GetAnonymousNodesFor is the 'this' I was referring to. We would need to do it, but that would break all XUL elements that call it, which I bet is a lot.
Comment 17•18 years ago
|
||
This site uses xbl (and getAnonymousNodes):
http://webfx.eae.net/dhtml/xblmarquee/xblmarquee.html
It's basically where I learned how to use xbl.
I used it myself a lot on http://www.kantjils.nl/ when I was the webmaster there.
I would be extremely unhappy, if xbl would be disabled.
Reporter | ||
Comment 18•18 years ago
|
||
(In reply to comment #12)
>
> And as a second question, what modifications would actually help the security
> situation?
>
stealing bindings and/or xml?
same origin checks?
Comment 19•18 years ago
|
||
> stealing bindings and/or xml?
Meaning what?
I'm not sure how same origin checks would help. Checks where?
Reporter | ||
Comment 20•18 years ago
|
||
(In reply to comment #19)
> > stealing bindings and/or xml?
>
> Meaning what?
>
> I'm not sure how same origin checks would help. Checks where?
>
meaning:
one visits http://evil which does '-moz-binding: url(http://secret/secret.xml)'
if secret.xml is secret binding may its source be read?
if secret.xml is just (valid) xml (not necessarily binding) may it be read?
does the spec say something about it?
Comment 21•18 years ago
|
||
Ah, I see.
I think the answer is mostly no to those questions (though you can get your hands on the anon content the binding clones and installs, if any).
Reporter | ||
Comment 22•18 years ago
|
||
so in the future the binding's source shouldn't be exposed?
Comment 23•18 years ago
|
||
Right.
Reporter | ||
Comment 24•18 years ago
|
||
Bug 371694 mentions "implement nsISecurityCheckedComponent"
the above bug is not very clear but is this possible from xbl?
implements="..."
Comment 25•18 years ago
|
||
only from privileged bindings
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/xbl/src/nsXBLContentSink.cpp&rev=1.78&mark=736-739#736
Assignee | ||
Comment 26•18 years ago
|
||
So it sounds like this bug is drifting into "make xbl suck less" rather than "kill xbl". If so we should close this bug and open bugs on specific issues?
Reporter | ||
Comment 27•18 years ago
|
||
(In reply to comment #26)
> So it sounds like this bug is drifting into "make xbl suck less" rather than
> "kill xbl". If so we should close this bug and open bugs on specific issues?
>
sure, close as you wish.
from my point of view |implements| is dead, the future of |extends| is unclear and |binding| is still alive :)
Reporter | ||
Comment 28•18 years ago
|
||
feel free to resolve as anyone wishes
Assignee | ||
Updated•18 years ago
|
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → WONTFIX
Reporter | ||
Comment 29•18 years ago
|
||
i dare propose a weaker version:
let disable user xbl/bindings in thunberbird - both in preview pane and in editor.
I really think we should remove support for -moz-binding from untrusted content. It's a vector to script execution that _no_ sanitizer expects to find, though of course someone out there uses it; someone uses everything, even our bugs. I think we should be willing to break the _tiny_ number of people using them in order to remove this attack surface. (Similarly for linking to chrome: from content, though that's another bug.)
A lesser alternative would be to restrict -moz-binding in content to strictly same origin loads (no data:, for example), which would reduce the threat of a -moz-binding being inserted against the site operator's knowledge.
Status: RESOLVED → REOPENED
Flags: blocking1.9?
OS: Linux → All
Hardware: PC → All
Resolution: WONTFIX → ---
Comment 31•17 years ago
|
||
I'm all for restricting it to same-origin by default... I really don't think we should neuter remote XBL altogether.
Comment 32•17 years ago
|
||
See also bug 324253, "Do something about the XSS issues -moz-binding introduces". But like Shaver, I'm more worried about crashes and memory safety than XSS. Rumor has it that it would be very hard to fix the known memory safety issues with XBL 1.
I don't think the compatibility argument holds water. We're going to get rid of XBL 1 at some point anyway when we replace it with the saner XBL 2, right?
Comment 33•17 years ago
|
||
> I really don't think we should neuter remote XBL altogether.
This needs to be backed up with some reasoning. We will take a Firefox-compat hit on a few sites if we do this--is that the concern?
Comment 34•17 years ago
|
||
Most of the sites I'm concerned about are not public-web but intranet: one of the lasting impressions I got from the Paris devday was how much XBL was in use on non-chrome intranet sites.
It might be sufficient for this purpose to disable remote-XBL by default and enable it via pref. But I really don't want to say that Le Monde just can't use Firefox 3 for their online editorial staff any more.
And boy, I wish we had brought this up sooner because I believe without good data that there's at least as much if not more XBL out on the web as MathML.
Can intranet sites not sign and elevate? Do we still support SSL-as-implicit-signature? A default pref off would be ok, site-specific for turn-on would be better. I really don't want to say that Le Monde published a front-page lolcat because they did use Firefox 3 with XBL enabled.
If you're not worried about the public web, then why worry about how much MathML there is? (I'd rather see us support MathML than XBL for content, though; at least there's a specification and I believe something resembling a test suite.)
(In reply to comment #33)
> I don't think the compatibility argument holds water. We're going to get rid
> of XBL 1 at some point anyway when we replace it with the saner XBL 2, right?
Going from XBL1 to *nothing* and then to XBL2 is a lot harsher than from XBL1 to XBL2.
Comment 37•17 years ago
|
||
> Can intranet sites not sign and elevate?
Not really, no, unless there happens to be JS on the stack while the XBL load is kicking off (so during frame construction, say).
I suppose we could add a caps thing to turn this on by hostname. But it might make sense to allow same-origin no matter what. It would be _really_ good to get some actual info on the uses. :(
> Do we still support SSL-as-implicit-signature?
I'm not sure what you mean, but I suspect the answer is "no" no matter what it is...
Reporter | ||
Comment 38•17 years ago
|
||
i continue to think that no version of XBL should be accessible from userland.
(the sanitizers arguments is not correct imo - sanitizers should use whitelist normalization, so they should not be vulnerable to constructs they don't know of).
sure some apps will break. so what? some users and developers will be pissed off. later they'll get over it. they will be more pissed off if they get owned constantly because of xbl features.
you can't please everyone.
Assignee | ||
Comment 39•17 years ago
|
||
Since it sounds like XBL is actually used out there I don't think we should remove it. Signing and things like that is a major headache, and not really something that I think we should encourage anyway. And if we're starting to have to put in a lot of effort to re-enabling XBL in the cases where we think it should still be permitted, we might as well put that effort into fixing the bugs we have with XBL instead.
(In reply to comment #41)
> Since it sounds like XBL is actually used out there I don't think we should
> remove it.
I don't understand this -- of course XBL is used out there, it's in a browser. boxObject is used out there too, and more widely, to say nothing of cross-document node movement, but we were willing to make authors update for those cases only for spec purity. Why would you blanket object to disabling content XBL on the basis of there being _some_ use?
(All the reasons in favour of boxObject and WRONG_DOCUMENT_ERR are ten times more applicable here, _plus_ security issues.)
We break things in _minor_ updates for security, like jar:, and expect people to cope, because it's for the greater good.
> Signing and things like that is a major headache, and not really
> something that I think we should encourage anyway.
Why don't you think we should encourage signing of code? (If we treat SSL-delivered content as signed, it gets a lot easier, though we would need some way to deal with bz's issue -- they could perhaps add the binding rules through the CSSOM from privileged script?)
> And if we're starting to
> have to put in a lot of effort to re-enabling XBL in the cases where we think
> it should still be permitted, we might as well put that effort into fixing the
> bugs we have with XBL instead.
I don't think we have a good handle on what bugs we do have, and the code is (very) hard to test.
I'm not talking about a lot of effort, I'm talking about disabling -moz-binding processing from content, maybe with a (site-specific) pref override. Le Monde can ship an extension or non-default pref, or whatever -- after they get their stuff working again from the constructor and other XBL changes we made on the trunk. Everyone else can sleep easier. (Do Le Monde have any Windows 98 machines? X with pseudocolour displays? Panther?)
Comment 41•17 years ago
|
||
I fully agree with shaver, even though TomTom HOME 2 *does* use XBL in content. I'd be happy to adapt to whatever is there, or even get rid of the current solution altogether, if that's improving Firefox security.
In fact, if we think it's a security hazard that cannot get fixed in the near future, I think it's mandatory to disable it.
Introducing a same-origin check will not help the surfer in that case either. I think a pref (ideally per site) that intranet users can enable would be a good compromise.
(In reply to comment #42)
> (In reply to comment #41)
> > Since it sounds like XBL is actually used out there I don't think we should
> > remove it.
>
> I don't understand this -- of course XBL is used out there, it's in a
> browser.
> boxObject is used out there too, and more widely, to say nothing of
> cross-document node movement, but we were willing to make authors update for
> those cases only for spec purity.
For both boxObject and node movement, there are simple local changes that authors can apply to fix their content. If we just disable XBL there are no simple changes that will get that content working again (other than policy changes that reenable XBL for that content, such as signing the content).
FWIW we haven't disabled boxObject for Web content in FF3, to give authors a transition period during which they can migrate to new APIs, especially getBoundingClientRect which we introduced in FF3 to handle the common use-case for boxObject.
Assignee | ||
Comment 43•17 years ago
|
||
We need to figure out something here
Assignee: xbl → jonas
Status: REOPENED → NEW
Flags: blocking1.9? → blocking1.9+
Priority: -- → P2
Reporter | ||
Comment 44•17 years ago
|
||
please at least give me a preference to disable remote xbl globally
Comment 45•17 years ago
|
||
Please see comment 45. Doing that is not enough to solve the problems. It just gives a false sense of security.
Reporter | ||
Comment 46•17 years ago
|
||
(In reply to comment #48)
> Please see comment 45.
someone has rm-ed this comment so fast it is not even in my mail
Comment 47•17 years ago
|
||
If you really do just want that, writing an extension that does it via content policy is pretty easy...
Comment 48•17 years ago
|
||
Comment 45 is marked as private. georgi, you're not in the security group on Bugzilla?
Reporter | ||
Comment 49•17 years ago
|
||
(In reply to comment #51)
> Comment 45 is marked as private. georgi, you're not in the security group on
> Bugzilla?
>
not in this group.
Comment 50•17 years ago
|
||
I'm injecting XBL into content documents with my Image Toolbar extension in order to capture image hover events and raise a custom event (by binding to images), which worked a lot better than trying to add event listeners all over the place.
If you're disabling XBL for untrusted content, presumably for 3.0, am I suddenly going to be faced with a large rewrite before the RCs, or will my method still work because it was added from chrome? If it has any relevance, currently my code fails if JavaScript is disabled due to bug 236839.
Comment 51•17 years ago
|
||
That's one of the use cases we're trying hard to not break.
Assignee | ||
Comment 52•17 years ago
|
||
Ben: Specifically how are you adding those bindings? Are you inserting CSS rules somehow or are you using .addBinding? Or some other way?
Comment 53•17 years ago
|
||
I'm currently inserting CSS bindings using nsIStyleSheetService loadAndRegisterSheet().
Comment 54•17 years ago
|
||
Jonas says this is WONTFIX.
Status: NEW → RESOLVED
Closed: 18 years ago → 17 years ago
Resolution: --- → WONTFIX
Comment 55•17 years ago
|
||
Can you please elaborate why? Last state of comments here is still the investigation.
Assignee | ||
Comment 56•17 years ago
|
||
So there are two categories of concerns:
1. XBL allows CSS to execute script.
2. We have exploitable crash bugs in the implementation.
So I realized 1 is an issue in IE too, which makes it slightly less bad. We're also going to tighten the same-origin policy better which will require that people not only that people can insert CSS rules, but also upload XBL files with an XML mimetype to the same server.
2 turned out to be not so bad. We have it mostly under control and have gotten pretty good testing on it. There is a set of bugs that needs to be fixed, but that should be possible to happen pretty quickly.
Additionally, it's turned out that XBL is in fact used in intranet apps. This makes me very reluctant to disable it. Especially with no real alternative.
The intranet-app case is why we proposed per-site pref machinery, no? I'm glad to hear that our XBL testing situation is in better shape than I'd thought.
Assignee | ||
Comment 58•17 years ago
|
||
So the main reason i was for the per-site opt-in was to deal with the issue 1 in comment 59. But given that IE also allows scripting via CSS, and we'd be able to block only using CSS to insert scripts (you'd have to upload the XBL as a separate file as well) i'm less worried.
Comment 59•17 years ago
|
||
(In reply to comment #61)
> But given that IE also allows scripting via CSS
I don't think that should be a factor in our decision.
Comment 60•17 years ago
|
||
> We're also going to tighten the same-origin policy better which will require that people not only that people can insert CSS rules, but also upload XBL files with an XML mimetype to the same server.
Does that mean we're going to break using data: URLs for XBL? I rely on that feature for testing XBL, so I'll want some kind of workaround. Some tests will need changes too: http://lxr.mozilla.org/mozilla/search?string=data%3Atext%2Fxml
(I think that feature was added in bug 366770, fwiw.)
Assignee | ||
Comment 61•17 years ago
|
||
Yes we will break data: urls, which means that it'll affect a pile of mochitests.
Would you be ok with a simple pref that'll enable data-urls again? Or would you be vary of turning that on since you could expose yourself to XSS attacks?
Comment 62•17 years ago
|
||
I'd be fine with using a pref.
Reporter | ||
Comment 63•17 years ago
|
||
btw, trunk allows bindings in the same file via data uri's and probably other ways in xhtml - there were some bugzilla testcases with the binding in the same xhtml, not sure if they still work
Comment 64•17 years ago
|
||
(In reply to comment #64)
> Yes we will break data: urls, which means that it'll affect a pile of
> mochitests.
Are you talking about breaking them completely, or just for "content"?
Assignee | ||
Comment 65•17 years ago
|
||
Why do you need it for chrome? Seems like most of the time it's useful for testing and nothing else, and I plan to add a pref that enables it for testing.
Reporter | ||
Comment 66•17 years ago
|
||
how does data: uris differ from this inline xbl?
<div style="-moz-binding: url(#randomxbl);">test</div>
#randomxbl is declared in the same file.
testcase to follow.
Reporter | ||
Comment 67•17 years ago
|
||
Assignee | ||
Comment 68•17 years ago
|
||
Yes, that'll work too, but only if the mimetype of the document is text/xml or application/xml (I plan on not making it work for application/xhtml+xml) which is a very rare type for content insertion attacks.
Reporter | ||
Comment 69•17 years ago
|
||
(In reply to comment #71)
> Yes, that'll work too, but only if the mimetype of the document is text/xml or
> application/xml (I plan on not making it work for application/xhtml+xml) which
> is a very rare type for content insertion attacks.
>
imho this is a kludge. so you are protecting text/html but not protecting text/xml ?
Reporter | ||
Comment 70•17 years ago
|
||
so i am completely missing why content insertion matters in this bug.
just make clear and document that "-moz-binding" allows content insertion just like "iframe" does and let CMS deal with their problems - they should strip "-moz-binding" the way they they strip "iframe" and "script" if their primary concern is injecting js/content.
Comment 71•17 years ago
|
||
I'm with georgi on this one. Do we need a new bug?
/be
Comment 72•17 years ago
|
||
Georgi's example relies on allowing users to upload XML files and then run script from them, right? You can already do that trivially: just stick an <html:script> in the XML. I don't think the XBL adds much to that vector... if you're allowing people to upload arbitrary XML and putting it on the web, you're allowing those scripts to run, period.
Reporter | ||
Comment 73•17 years ago
|
||
>2 turned out to be not so bad. We have it mostly under control and have gotten
>pretty good testing on it.
being bug spammed by Bug 378518 i conjecture the above statement is completely wrong.
Comment 74•13 years ago
|
||
This was WONTFIXed, but eventually done as part of bug 546857.
http://mxr.mozilla.org/mozilla-central/search?string=allowxulxbl
Resolution: WONTFIX → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•