Last Comment Bug 641342 - Same-origin policy not applied when chrome: resources are loaded via a custom protocol handler
: Same-origin policy not applied when chrome: resources are loaded via a custom...
Status: VERIFIED FIXED
[sg:moderate] fixed-in-tracemonkey [a...
: regression
Product: Core
Classification: Components
Component: XPConnect (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla6
Assigned To: Blake Kaplan (:mrbkap)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on: 657292
Blocks: 597162
  Show dependency treegraph
 
Reported: 2011-03-13 10:57 PDT by Felix Kling
Modified: 2013-12-27 14:25 PST (History)
16 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed
.x+
wanted
unaffected
unaffected


Attachments
The plugin which provides a custom protocol to demonstrate the problem (3.35 KB, application/x-xpinstall)
2011-03-13 10:59 PDT, Felix Kling
no flags Details
Proposed fix (3.01 KB, patch)
2011-03-16 14:49 PDT, Blake Kaplan (:mrbkap)
no flags Details | Diff | Splinter Review
Proposed fix (3.01 KB, patch)
2011-03-21 18:22 PDT, Blake Kaplan (:mrbkap)
gal: review+
jst: review+
jst: approval‑mozilla‑beta+
Details | Diff | Splinter Review
Fixes for mochitests (4.60 KB, patch)
2011-05-11 14:55 PDT, Blake Kaplan (:mrbkap)
gal: review+
Details | Diff | Splinter Review
Fix + bustage fix (8.17 KB, patch)
2011-06-03 14:46 PDT, Blake Kaplan (:mrbkap)
mrbkap: review+
Details | Diff | Splinter Review

Description User image Felix Kling 2011-03-13 10:57:47 PDT
User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; en-US) AppleWebKit/534.16 (KHTML, like Gecko) Chrome/10.0.648.133 Safari/534.16
Build Identifier: Firefox 4: 20110303140758

When a custom protocol handler loads a chrome: resource, the same-origin policy is not applied.

The flags of the protocol handler are only set to URI_LOADABLE_BY_ANYONE.

If the custom URL is loaded via an iframe which is injected into some webpage, this webpage has access to the iframes DOM.

This only seems to be the case if the protocol handler loads a resource from an internal URL (e.g. chrome:) but if it loads an external webpage.

This problem does not occur in Firefox 3.6, but the behaviour is the same if both flags, URI_LOADABLE_BY_ANYONE and URI_INHERITS_SECURITY_CONTEXT, are set.

Reproducible: Always

Steps to Reproduce:
1. Install the the simple plugin attached to this report. It just includes a basic HTML file and a protocol handler implementation that registers the custom: protocol.

2. Create a bookmarklet with this code:

javascript:(function(){f=document.createElement('iframe');f.src="custom:blank";f.onload=function(){alert(this.contentDocument.body.innerHTML)};document.body.appendChild(f);}())

This will inject an iframe into the current webpage and try to access the iframe's content after loading.

3. Go to any website and execute the bookmarklet.
Actual Results:  
The content of the iframe is "alerted".

Expected Results:  
A *Permission denied* error should be raised, like in Firefox 3.6. ( Error: Permission denied for <...> to get property HTMLDocument.body)
Comment 1 User image Felix Kling 2011-03-13 10:59:30 PDT
Created attachment 519020 [details]
The plugin which provides a custom protocol to demonstrate the problem
Comment 2 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-14 06:42:00 PDT
Blake, what's going on here?

The principal of the document in that subframe is the system principal.  If I try to get location.href from it, that throws, but document.documentURI, say, works fine from the untrusted script....

Reporter, just to be clear, your protocol handler has a security bug.  It allows anyone to load privileged pages.  This has all sorts of issues in Firefox 3.6 as well.  You need to fix that.
Comment 3 User image Daniel Veditz [:dveditz] 2011-03-15 10:54:26 PDT
(In reply to comment #2)
> Reporter, just to be clear, your protocol handler has a security bug.  It
> allows anyone to load privileged pages.  This has all sorts of issues in
> Firefox 3.6 as well.  You need to fix that.

Let me heartily second that. It's hard to suggest a fix without knowing what you're trying to accomplish. If the framed pages need the chrome privilege to function then you need to do something completely different from framing. In other words, if you've created your custom protocol to get around the restriction on pages loading chrome URLs you've already lost: those restrictions are there for very good reasons. If your frames are just visual resources then you could maybe allow the framing and instead create data: uris or use resource:. Or null out the owner explicitly to de-privilege the chrome channel as Boris mentioned in the newsgroup. You could look at how we've done some of our UN-privileged about: pages where the scheme loads the basic structure but privileged code outside the page adds the necessary information or responds to trusted events.

This bug is not about that though. For help on restructuring your addon please see the newsgroups. Probably mozilla.dev.extensions will be more help than the .security group.

This bug is about the same-origin check failure for custom protocols, which could lead to security bugs in addons. If somehow it applies to content-specified protocol handlers then it might not even require an add-on. Will try that in a moment.
Comment 4 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-03-15 11:07:12 PDT
Don't think we'd block Firefox 4 for this, but I'd entertain a branch fix. Is this new from 3.6?
Comment 5 User image Johnathan Nightingale [:johnath] 2011-03-15 11:21:33 PDT
Discussed in triage today - we'd like to see this fixed, but the steps required to exploit a user seem to be at least as high as getting them to download and run an executable -- not respinning 4.0 for it
Comment 6 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 11:40:36 PDT
To be clear, the failure here is that the untrusted webpage is being considered same-origin in at least some cases with a page that has the system principal.  The custom protocol is only relevant insofar as it sticks a system-principal page somewhere where an untrusted page can get at it.

Here are some steps to reproduce that do not involve any custom protocols:

1)  Pick a webpage of your choice.
2)  On that page, run this from the url bar:
   
      javascript:var w = window.open(); void(0);
3)  In the tab that opens, load about:addons
4)  Back on the page from step 1, run this from the url bar:

      javascript:alert(w.document.documentElement.textContent)

Observer the fact that the untrusted page has access to the DOM in the about:addons window, and cry.

No addons required.  A bit of social-engineering needed.  I'm not sure whether the lack of a same-origin check there can be used to run code, or is just a data leak.
Comment 7 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-03-15 11:42:29 PDT
Renominating based on above, but I still think the social engineering requirement makes this a .x not a final+
Comment 8 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 11:46:09 PDT
Oh, and:

5)  Run this in the url bar in the window from step 1:

      javascript:alert(w.isContentFrame(w))

and notice that the function is called.  Or try this:

      javascript:try { w.urlSecurityCheck(5); } catch(e) { alert(e); }

which shows that we're calling a function which has system privileges, since it gets past the getService call for the security manager.
Comment 9 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 11:47:39 PDT
So a typical attack might look like this.  Create a page which has advice to make Firefox faster.  Tell the user to click in the url bar and type about:config to get to the configuration stuff.  As soon as they do that, you can call whatever functions are defined in about:config.

It's not an obvious exploit yet, but I bet moz_bug_r_a4 can make it so!
Comment 10 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 11:50:57 PDT
Oh, and I'm 99.99% sure this is a compartments regression.  3.6 definitely doesn't have this sort of behavior!
Comment 11 User image Felix Kling 2011-03-15 11:57:23 PDT
(In reply to comment #3)
> If your frames are just visual
> resources then you could maybe allow the framing and instead create data: uris
> or use resource:. Or null out the owner explicitly to de-privilege the chrome
> channel as Boris mentioned in the newsgroup. You could look at how we've done
> some of our UN-privileged about: pages where the scheme loads the basic
> structure but privileged code outside the page adds the necessary information
> or responds to trusted events.

Just to explain the context (and this is all I want to say about it): I am adding user data to webpages but it should not be accessible by the parent page. I thought I could do this by loading an iframe with a different URL (=> same origin policy prevents access). The file I load is only a placeholder. I don't want to run privileged code in it.
I will have a look at the about: URLs, this sounds like what I want. Thanks for the help!

I just created an addon as testcase, because I couldn't reproduce it in an other way. In my naiveness I thought it might be related with the flags of the protocol handler. I'm sorry, I have not so much insight in the inner workings.

Hope you guys can make sense out of it and figure it out!
Comment 12 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 12:02:19 PDT
fkling, your expectation about same-origin policies was fine.  This bug is about the fact that the policy is broken in this case.  ;)
Comment 13 User image Daniel Veditz [:dveditz] 2011-03-15 13:47:50 PDT
(In reply to comment #11)
> I will have a look at the about: URLs, this sounds like what I want. Thanks for
> the help!

To be clear I wasn't suggesting that you create a new "about:" url for yourself, but rather than you look at how those were done (say, about:neterror) and do something similar. I suspect you might be able to make resource: work (ask in the extension developer group).
Comment 14 User image moz_bug_r_a4 2011-03-15 20:02:54 PDT
(In reply to comment #9)
> So a typical attack might look like this.  Create a page which has advice to
> make Firefox faster.  Tell the user to click in the url bar and type
> about:config to get to the configuration stuff.  As soon as they do that, you
> can call whatever functions are defined in about:config.
> 
> It's not an obvious exploit yet, but I bet moz_bug_r_a4 can make it so!

It seems that the problem you are talking about is basically bug 597162 comment
#9, and a working exploit is testcase 4 in bug 597162.
Comment 15 User image Boris Zbarsky [:bz] (still a bit busy) 2011-03-15 20:08:24 PDT
Hrm..  Yes, that seems plausible.
Comment 16 User image Blake Kaplan (:mrbkap) 2011-03-16 01:45:48 PDT
So, I think I have a plan here. I'm going to try it tomorrow. Basically, I'm going to try giving chrome DOM objects XOWs (by which I mean their modern, unnamed, equivalent) instead of COWs.
Comment 17 User image Mike Beltzner [:beltzner, not reading bugmail] 2011-03-16 11:38:35 PDT
.x because it's not drive-by exploitable, but definitely should fix.
Comment 18 User image Blake Kaplan (:mrbkap) 2011-03-16 14:49:30 PDT
Created attachment 519783 [details] [diff] [review]
Proposed fix

This seems to work. It also fixes bug 597162.
Comment 19 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-03-18 09:30:05 PDT
Comment on attachment 519783 [details] [diff] [review]
Proposed fix

+        JSObject *inner = obj;
+        OBJ_TO_INNER_OBJECT(cx, obj);

You want inner here, not obj, right?
Comment 20 User image Blake Kaplan (:mrbkap) 2011-03-21 18:22:22 PDT
Created attachment 520824 [details] [diff] [review]
Proposed fix

Oops, yes.
Comment 21 User image Andreas Gal :gal 2011-03-21 22:06:00 PDT
Comment on attachment 520824 [details] [diff] [review]
Proposed fix

We should common out the holder creation code. And the holder indeed wants obj, not inner, right?
Comment 22 User image Blake Kaplan (:mrbkap) 2011-04-08 15:10:54 PDT
http://hg.mozilla.org/mozilla-central/rev/652bd0eed005
Comment 23 User image Mats Palmgren (:mats) 2011-04-09 07:34:03 PDT
Reopening since the fix was backed out:
http://hg.mozilla.org/mozilla-central/rev/d1d5daab95bd
Comment 24 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-04-28 13:38:49 PDT
Blake, what's our plan forward here?
Comment 25 User image Marcia Knous [:marcia - use ni] 2011-05-05 13:23:34 PDT
Following up on jst's comment - do we have an updated status?  Thanks.
Comment 26 User image Blake Kaplan (:mrbkap) 2011-05-05 18:18:34 PDT
I have to debug the mochitest failures. They both had to do with UniversalXPConnect, so hopefully the fix won't be too difficult to implement.
Comment 27 User image Blake Kaplan (:mrbkap) 2011-05-11 14:55:52 PDT
Created attachment 531761 [details] [diff] [review]
Fixes for mochitests

This applies on top of attachment 520824 [details] [diff] [review]. Basically, for former XOWs (which are now simply FilteringWrappers on top of Xray wrapeprs) we don't have .wrappedJSObject and we shouldn't try to resolve it. This also fixes a couple of tests that were expecting XPCNativeWrappers when they shouldn't have been.

The .xul test was trying to actually use this bug to access a chrome window from content, so I made the "content" actually be chrome.
Comment 28 User image Blake Kaplan (:mrbkap) 2011-05-12 13:56:54 PDT
http://hg.mozilla.org/tracemonkey/rev/673f93bb84aa
Comment 29 User image Asa Dotzler [:asa] 2011-05-18 13:50:58 PDT
Is this a candidate for uplift into Firefox 5 Beta? What's the risk? If it is a Beta candidate then can you please ask for beta approval in the patch with a risk/reward?
Comment 30 User image Asa Dotzler [:asa] 2011-05-18 13:56:08 PDT
Boris notes that this patch fixes also tracking-firefox5+ bug 597162.
Comment 31 User image Daniel Veditz [:dveditz] 2011-05-19 13:33:52 PDT
We should cherry pick this patch over to mozilla-central if there's no TM merge planned in the near future (there should be, though, right?).
Comment 32 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-05-24 14:37:17 PDT
mrbkap, I think we should take the change that landed for this bug (and others) for Firefox 5. Do you agree?
Comment 33 User image Asa Dotzler [:asa] 2011-05-26 13:27:18 PDT
A risk analysis and a description of any mitigation (testing, locality of fix, etc.) that would help give us confidence in taking this late in Firefox 5 would be really helpful.
Comment 34 User image Asa Dotzler [:asa] 2011-05-31 18:12:48 PDT
marking this as fixed in Firefox 6 since it made the migration. Now we just need to get it into Beta for 5 if that's appropriate.
Comment 35 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-06-01 12:20:59 PDT
Comment on attachment 520824 [details] [diff] [review]
Proposed fix

Approved for 5 beta, please land asap.
Comment 36 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-06-01 15:57:06 PDT
http://hg.mozilla.org/releases/mozilla-beta/rev/54f406860c83

Closing this bug as this is fixed for 5, and it landed on trunk before we branched for 6, so I don't think there's anything left to do here.
Comment 37 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2011-06-01 19:56:41 PDT
This made test_leaf_layers_partition_browser_window.xul permaorange:
http://tinderbox.mozilla.org/showlog.cgi?log=Mozilla-Beta/1306974341.1306979779.22919.gz&fulltext=1

So I backed it out:
http://hg.mozilla.org/releases/mozilla-beta/rev/14704e7a7899

I'm not sure what the problem is. If you figure out what the problem is, I'd be OK with you disabling that test on beta.
Comment 38 User image Blake Kaplan (:mrbkap) 2011-06-03 14:46:59 PDT
Created attachment 537241 [details] [diff] [review]
Fix + bustage fix

So, it turned out that there was a bustage fix for this already on trunk + tracemonkey (changeset 5ff15fe83e16). So, this patch includes that fix.
Comment 40 User image Johnny Stenback (:jst, jst@mozilla.com) 2011-06-03 15:13:56 PDT
Fix + bustage fix landed on beta:

http://hg.mozilla.org/releases/mozilla-beta/rev/ef6beddbb71d
Comment 41 User image Robert Kaiser 2011-06-06 10:35:45 PDT
(In reply to comment #40)
> Fix + bustage fix landed on beta:

Doesn't bug 657292 need to land on beta as well, then to not make this crash?
Comment 42 User image Blake Kaplan (:mrbkap) 2011-06-06 11:41:27 PDT
Yes... I landed it just now.
Comment 43 User image Daniel Veditz [:dveditz] 2011-06-10 12:29:24 PDT
Lowering the severity of this bug *as described in the summary* to moderate -- user would have to install such a protocol handler. The worse effects described in comment 6 are the common cause but a duplicate of bug 597162 as noted in comment 14 (which is critical, to be sure).
Comment 45 User image George Carstoiu 2011-07-20 04:22:55 PDT
I checked this issue using the steps from Comment 6:
 1. Pick a webpage of your choice.
 2. On that page, run this in the webconsole: javascript:var w = window.open(); void(0);
 3.  In the tab that opens, load about:addons
 4. Back on the page from step 1, run this in the webconsole: javascript:alert(w.document.documentElement.textContent) 

Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0  - beta 2:
On Windows and Linux builds: Error: Permission denied to access property 
On Mac the alert message displays the content text from the about:addons page

Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110719 Firefox/8.0a1
Error: Permission denied to access property 

Shouldn't the 6.0b2 Mac build display the Permission denied error also? Although the problem may be reproducible on 6.0b2 it is no longer on the Nightly build.

Should the patch be pushed again only for the Mac branch?
Comment 46 User image Boris Zbarsky [:bz] (still a bit busy) 2011-07-20 08:36:10 PDT
> Shouldn't the 6.0b2 Mac build display the Permission denied error also?

Yes.

> Should the patch be pushed again only for the Mac branch?

There is no "Mac branch".  The code for 6.0b2 is the same for all OSes.  Could you please double-check what build you tested this with on Mac?
Comment 47 User image Vlad [QA] 2011-08-11 05:50:44 PDT
Setting resolution to Verified Fixed on Mozilla/5.0 (Windows NT 6.1; rv:6.0) Gecko/20100101 Firefox/6.0 beta 5 
and also on
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0) Gecko/20100101 Firefox/6.0 beta 5

Error: Permission denied to access property appears also on Mac.

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