3.35 KB, application/x-xpinstall
3.01 KB, patch
|Details | Diff | Splinter Review|
4.60 KB, patch
|Details | Diff | Splinter Review|
8.17 KB, patch
|Details | Diff | Splinter Review|
Created attachment 519020 [details] The plugin which provides a custom protocol to demonstrate the problem
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.
(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.
Don't think we'd block Firefox 4 for this, but I'd entertain a branch fix. Is this new from 3.6?
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
Renominating based on above, but I still think the social engineering requirement makes this a .x not a final+
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!
Oh, and I'm 99.99% sure this is a compartments regression. 3.6 definitely doesn't have this sort of behavior!
(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!
fkling, your expectation about same-origin policies was fine. This bug is about the fact that the policy is broken in this case. ;)
(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).
(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.
Hrm.. Yes, that seems plausible.
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.
.x because it's not drive-by exploitable, but definitely should fix.
Created attachment 519783 [details] [diff] [review] Proposed fix This seems to work. It also fixes bug 597162.
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?
Created attachment 520824 [details] [diff] [review] Proposed fix Oops, yes.
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?
Reopening since the fix was backed out: http://hg.mozilla.org/mozilla-central/rev/d1d5daab95bd
Blake, what's our plan forward here?
Following up on jst's comment - do we have an updated status? Thanks.
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.
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.
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?
Boris notes that this patch fixes also tracking-firefox5+ bug 597162.
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?).
mrbkap, I think we should take the change that landed for this bug (and others) for Firefox 5. Do you agree?
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.
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 on attachment 520824 [details] [diff] [review] Proposed fix Approved for 5 beta, please land asap.
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.
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.
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.
Fix + bustage fix landed on beta: http://hg.mozilla.org/releases/mozilla-beta/rev/ef6beddbb71d
(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?
Yes... I landed it just now.
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).
> 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?
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.