641342 - Same-origin policy not applied when chrome: resources are loaded via a custom protocol handler

Status: VERIFIED FIXED

(Core :: XPConnect, defect)

Description

[Felix Kling]

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

[Felix Kling]

Attachment: The plugin which provides a custom protocol to demonstrate the problem

Comment 2

[Boris Zbarsky [:bzbarsky]]

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

[Daniel Veditz [:dveditz]]

(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

[Mike Beltzner [:beltzner, not reading bugmail]]

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

[Johnathan Nightingale [:johnath]]

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

[Boris Zbarsky [:bzbarsky]]

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

[Mike Beltzner [:beltzner, not reading bugmail]]

Renominating based on above, but I still think the social engineering requirement makes this a .x not a final+

Comment 8

[Boris Zbarsky [:bzbarsky]]

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

[Boris Zbarsky [:bzbarsky]]

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

[Boris Zbarsky [:bzbarsky]]

Oh, and I'm 99.99% sure this is a compartments regression.  3.6 definitely doesn't have this sort of behavior!

Comment 11

[Felix Kling]

(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

[Boris Zbarsky [:bzbarsky]]

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

[Daniel Veditz [:dveditz]]

(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

[moz_bug_r_a4]

(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

[Boris Zbarsky [:bzbarsky]]

Hrm..  Yes, that seems plausible.

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

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

[Mike Beltzner [:beltzner, not reading bugmail]]

.x because it's not drive-by exploitable, but definitely should fix.

Comment 18

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

This seems to work. It also fixes bug 597162.

Comment 19

[Johnny Stenback (:jst)]

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

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

Oops, yes.

Comment 21

[Andreas Gal :gal]

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

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/mozilla-central/rev/652bd0eed005

Comment 23

[Mats Palmgren (inactive)]

Reopening since the fix was backed out:
http://hg.mozilla.org/mozilla-central/rev/d1d5daab95bd

Comment 24

[Johnny Stenback (:jst)]

Blake, what's our plan forward here?

Comment 25

[Marcia Knous [:marcia]]

Following up on jst's comment - do we have an updated status?  Thanks.

Comment 26

[Blake Kaplan (:mrbkap) (inactive)]

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

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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

[Blake Kaplan (:mrbkap) (inactive)]

http://hg.mozilla.org/tracemonkey/rev/673f93bb84aa

Comment 29

[Asa Dotzler [:asa]]

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

[Asa Dotzler [:asa]]

Boris notes that this patch fixes also tracking-firefox5+ bug 597162.

Comment 31

[Daniel Veditz [:dveditz]]

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

[Johnny Stenback (:jst)]

mrbkap, I think we should take the change that landed for this bug (and others) for Firefox 5. Do you agree?

Comment 33

[Asa Dotzler [:asa]]

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

[Asa Dotzler [:asa]]

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

[Johnny Stenback (:jst)]

Comment on attachment 520824 [details] [diff] [review]
Proposed fix

Approved for 5 beta, please land asap.

Comment 36

[Johnny Stenback (:jst)]

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

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: 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

[Johnny Stenback (:jst)]

Fix + bustage fix landed on beta:

http://hg.mozilla.org/releases/mozilla-beta/rev/ef6beddbb71d

Comment 41

[Robert Kaiser]

(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

[Blake Kaplan (:mrbkap) (inactive)]

Yes... I landed it just now.

Comment 43

[Daniel Veditz [:dveditz]]

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

[George Carstoiu]

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

[Boris Zbarsky [:bzbarsky]]

> 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

[Vlad [QA]]

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.