We need to add same-origin checks when loading overlays. IMHO a checkLoadURI (which we don't have either AFAICT) isn't enough since the loaded data will be inserted into the documents DOM and can therefor be read.

Why would same origin checks be required for overlays? Do you mean one http overlay trying to load another http overlay?

yes, that or one http xulpage loading overlays from another http site or an overlay on the local filesystem. This is a cumbersome, but still possible, way of reading data from places where you shouldn't be allowd to.

So someone can read local files with this, by opening a XUL IFRAME which loads an overlay from some local file URL? Sounds very bad.

This is listed as a 1.2 blocker. What's the status here?

Hyatt, we really want this for 1.2. Can you help us out?

Actually they could only read local files if the merge of the overlay is successful, and even then they'd only be able to read the portions that successfully merged with the XUL document. It sounds like can be fixed by adding a checkLoadURI call comparing the master doc's url to the overlay's url.

What happens when an overlay file is not found? Can someone use this to detect the existence of certain kinds of files on the local file system by checking for errors?

We're trying to evaluate this for 1.2. Is this serious or not? The risk is still unclear to my little mind. Please use large, easy to read type with no big words.

This bug makes it possible to read XML files from anywhere (including file:// and other webservers). However I think that the XML files must: 1. contain id attributes (fairly common) 2. use the XUL namespace (uncommon) But I'm not sure that 2 is a requirement.

oh, and we need to use checkSameOrigin since checkLoadURI allows other webservers

Is it possible to get this fixed for 1.3a, next Tuesday?

hyatt: we think this should be fixed, but I'd like your input on whether it should be checkLoadURI or checkSameOrigin (disallow overlaying XUL from other servers).

-> me.

Can we get this fixed by 1.4 beta?

I don't see any reason to disallow loading XUL overlays from another server. Does anyone have a good argument for that?

Doesn't reading overlays from any server allow you to read a lot of xml-data from any server. All you have to do is to set up an element with an id that you are interested in and you'll get the entire subtree with that id cloned into your document, allowing you read it using the DOM.

My initial response was "if you can get it through overlays, you can get it using telnet". But there's this thing about you loading some XUL document on your machine inside a firewall, and then that XUL document has access to servers inside the firewall, something the attacker (author of the XUL document outside the firewall) doesn't have with telnet. Then all you need to do is get the information back out (e.g. hook it off a link to http://www.attackersdomain.com/getPage.cgi?your_internal_data_goes_here returning some nice page)

Loading non-chrome overlays has actually been broken for some time now. This patch fixes that and also adds the same-origin check.

Exactly, this is the same reason why we dissallow loading files from other servers using XMLHttpRequest, document.load, etc. There's also the issue that you could get stuff from passwordprotected servers where the clients browser will automatically log in, like banks and such.

Comment on attachment 122238 [details] [diff] [review] patch don't you need to check for same-uri-ness even if the document is in the xulcache?

This tries to load an overlay from http://brianryner.com/, which should fail.

Note that trying to load the testcase will hang a build without this patch, due to the brokenness of loading non-chrome overlays that I pointed out.

The only thing that can be in the XUL cache is a chrome:// document; I don't see why we'd care if you overlay that.

Oh, but that does create an inconsistency, since the behavior would be different depending on whether the overlay in question had previously been loaded. New patch coming up.

Make the check independent of the XUL cache. Also, removed an assertion that is now impossible to trigger.

Comment on attachment 122244 [details] [diff] [review] patch Sigh, this one has problems too. We have to at least let chrome://foo/bar.xul load chrome://bar/foo.xul as an overlay or lots of stuff breaks.

This patch uses the following method for determining whether an overlay is allowed to load: - Chrome documents can load overlays from anywhere - Any document can load a chrome:// overlay - Otherwise, the master document and overlay must have the same origin

Comment on attachment 122250 [details] [diff] [review] better patch cool. sr=ben@netscape.com

Comment on attachment 122250 [details] [diff] [review] better patch Looks perfect. r=mstoltz. I'll take the liberty of requesting approval. Drivers: this patch fixes a hole that could allow an attacker to read some XML files from within a firewall or password-protected site. Since the security check we added makes an exception for chrome (only nonchrome pages loading nonchrome overlays are checked), the risk is low.

Comment on attachment 122250 [details] [diff] [review] better patch a=asa (on behalf of drivers) for checkin to 1.4b

checked in.

Bugs published on the Known-vulnerabilities page long ago, removing confidential flag.