Last Comment Bug 159450 - Need same origin checks for overlays
: Need same origin checks for overlays
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla1.4beta
Assigned To: Brian Ryner (not reading)
: Neil Deakin
Depends on:
  Show dependency treegraph
Reported: 2002-07-25 15:08 PDT by Jonas Sicking (:sicking) No longer reading bugmail consistently
Modified: 2008-07-31 03:09 PDT (History)
13 users (show)
asa: blocking1.4b+
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.62 KB, patch)
2003-05-01 15:18 PDT, Brian Ryner (not reading)
no flags Details | Diff | Splinter Review
testcase for cross-server overlay loading (233 bytes, application/vnd.mozilla.xul+xml)
2003-05-01 15:23 PDT, Brian Ryner (not reading)
no flags Details
patch (1.72 KB, patch)
2003-05-01 15:59 PDT, Brian Ryner (not reading)
no flags Details | Diff | Splinter Review
better patch (2.83 KB, patch)
2003-05-01 16:35 PDT, Brian Ryner (not reading)
security-bugs: review+
bugs: superreview+
asa: approval1.4b+
Details | Diff | Splinter Review

Description Jonas Sicking (:sicking) No longer reading bugmail consistently 2002-07-25 15:08:49 PDT
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.
Comment 1 David Hyatt 2002-08-04 19:44:53 PDT
Why would same origin checks be required for overlays? Do you mean one http overlay 
trying to load another http overlay?

Comment 2 Jonas Sicking (:sicking) No longer reading bugmail consistently 2002-08-04 20:58:47 PDT
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.
Comment 3 Robert O'Callahan (:roc) (email my personal email if necessary) 2002-10-13 22:10:50 PDT
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.
Comment 4 Christopher Blizzard (:blizzard) 2002-10-25 10:02:47 PDT
This is listed as a 1.2 blocker.  What's the status here?
Comment 5 Asa Dotzler [:asa] 2002-11-01 11:13:10 PST
Hyatt, we really want this for 1.2. Can you help us out?
Comment 6 David Hyatt 2002-11-06 15:33:30 PST
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.
Comment 7 Robert O'Callahan (:roc) (email my personal email if necessary) 2002-11-07 06:41:54 PST
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
Comment 8 Christopher Blizzard (:blizzard) 2002-11-13 10:31:52 PST
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.
Comment 9 Jonas Sicking (:sicking) No longer reading bugmail consistently 2002-11-14 09:32:45 PST
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. 
Comment 10 Jonas Sicking (:sicking) No longer reading bugmail consistently 2002-11-14 09:34:13 PST
oh, and we need to use checkSameOrigin since checkLoadURI allows other webservers 
Comment 11 Mitchell Stoltz (not reading bugmail) 2002-11-25 16:37:26 PST
Is it possible to get this fixed for 1.3a, next Tuesday?
Comment 12 jag (Peter Annema) 2003-03-06 15:32:29 PST
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
Comment 13 Brian Ryner (not reading) 2003-03-14 19:12:17 PST
-> me.
Comment 14 Mitchell Stoltz (not reading bugmail) 2003-04-10 16:15:08 PDT
Can we get this fixed by 1.4 beta?
Comment 15 Brian Ryner (not reading) 2003-05-01 12:02:30 PDT
I don't see any reason to disallow loading XUL overlays from another server. 
Does anyone have a good argument for that?
Comment 16 Jonas Sicking (:sicking) No longer reading bugmail consistently 2003-05-01 12:23:41 PDT
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.
Comment 17 jag (Peter Annema) 2003-05-01 15:07:15 PDT
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
returning some nice page)
Comment 18 Brian Ryner (not reading) 2003-05-01 15:18:07 PDT
Created attachment 122238 [details] [diff] [review]

Loading non-chrome overlays has actually been broken for some time now.  This
patch fixes that and also adds the same-origin check.
Comment 19 Jonas Sicking (:sicking) No longer reading bugmail consistently 2003-05-01 15:20:58 PDT
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 20 Jonas Sicking (:sicking) No longer reading bugmail consistently 2003-05-01 15:22:42 PDT
Comment on attachment 122238 [details] [diff] [review]

don't you need to check for same-uri-ness even if the document is in the
Comment 21 Brian Ryner (not reading) 2003-05-01 15:23:07 PDT
Created attachment 122239 [details]
testcase for cross-server overlay loading

This tries to load an overlay from, which should fail.
Comment 22 Brian Ryner (not reading) 2003-05-01 15:24:11 PDT
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.
Comment 23 Brian Ryner (not reading) 2003-05-01 15:41:36 PDT
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.
Comment 24 Brian Ryner (not reading) 2003-05-01 15:49:11 PDT
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.
Comment 25 Brian Ryner (not reading) 2003-05-01 15:59:34 PDT
Created attachment 122244 [details] [diff] [review]

Make the check independent of the XUL cache.  Also, removed an assertion that
is now impossible to trigger.
Comment 26 Brian Ryner (not reading) 2003-05-01 16:05:33 PDT
Comment on attachment 122244 [details] [diff] [review]

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.
Comment 27 Brian Ryner (not reading) 2003-05-01 16:35:48 PDT
Created attachment 122250 [details] [diff] [review]
better patch

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 28 Ben Goodger (use ben at mozilla dot org for email) 2003-05-01 16:39:06 PDT
Comment on attachment 122250 [details] [diff] [review]
better patch

Comment 29 Mitchell Stoltz (not reading bugmail) 2003-05-01 18:10:53 PDT
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 30 Asa Dotzler [:asa] 2003-05-01 18:34:36 PDT
Comment on attachment 122250 [details] [diff] [review]
better patch

a=asa (on behalf of drivers) for checkin to 1.4b
Comment 31 Brian Ryner (not reading) 2003-05-02 11:15:54 PDT
checked in.
Comment 32 Daniel Veditz [:dveditz] 2004-07-20 04:46:49 PDT
Bugs published on the Known-vulnerabilities page long ago, removing confidential

Note You need to log in before you can comment on or make changes to this bug.