Bugzilla@Mozilla – Bug 453310
XPCNativeWrapper pollution using top-level statement of chrome JS
Last modified: 2010-02-24 06:23:23 PST
Summon comment box
User-Agent: Build Identifier: Please see bug 444077 and bug 441087. When a top-level statement is executed, fp->callee is null. Thus, it's possible to circumvent the fix in bug 441087. Reproducible: Always
Created attachment 336487 [details] testcase 1 This works on trunk and fx3.0.x.
Created attachment 336488 [details] testcase 2 This works on fx2.0.0.x.
Not going to block 1.9.1 on this after all, but Blake, please do investigate!
Created attachment 344195 [details] [review] Proposed fix I'm a little sad that nsScriptSecurityManager is peeking into xpcconvert here but avoiding it is more work than it's worth. The core of the fix is the else branch just before the call to GetNewOrUsed where we pluck the script principal out if we didn't get a function object callee.
http://hg.mozilla.org/mozilla-central/rev/b91e44d05452
Since we're apparently opposed to taking bug 459906 on the 1.8 branch, there is no reason that this bug should block 1.8.1.18. I'll write up the backport patch for this bug soon, though, so it's sure to make the 1.8.1.19 and 1.9.0.5 releases.
Yeah, we talked and took bug 459906 instead of this one for this release. This bug blocks 1.8.1.19/1.9.0.5 however (so don't stop working on it!).
Comment on attachment 344195 [details] [review] Proposed fix This applies as-is to the 1.9 branch.
Comment on attachment 344195 [details] [review] Proposed fix Approved for 1.9.0.5, a=dveditz for release-drivers We need a separate 1.8.1.x patch, right?
Created attachment 348685 [details] [review] Patch for the 1.8 branch This fix is different from the one checked in on trunk. In particular, on the branch, it is much more expensive to get one's hands on an nsIScriptSecurityManager so it makes sense to do a little bit more work in XPCNativeWrapper since it has a script security manager already. I was trying to avoid this on trunk, since IMO this signature is ugly and harder to use.
Comment on attachment 348685 [details] [review] Patch for the 1.8 branch Oops, this is a patch for the 1.8 branch.
Fixed on the 1.9 branch.
Comment on attachment 348685 [details] [review] Patch for the 1.8 branch Approved for 1.8.1.19, a=dveditz for release-drivers
Fixed on the 1.8 branch.
Verified for 1.8.1.19 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.19pre) Gecko/2008112503 BonEcho/2.0.0.19pre. Verified for 1.9.0.5 with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.0.5pre) Gecko/2008112505 GranParadiso/3.0.5pre.
I think there is a problem with this patch. I'm not sure if this has any security implication though. If you load either of the attached testcases in the 3.0.5 beta, then double click in the page, when you bring up the context menu (right click), it's been corrupted, containing only: copy, select all, this frame >, and selection source. There are other odd things going on as well, as if you try to say highlight some text on the page, you just end up trying to drag and drop the iframe.
On fx3.0.x, when I double click in the page, the browser selects an empty <html> element and an <iframe> element in the subframe. It seems that the behavior you mentioned is the right behavior in this situation. (Even when JavaScript is disabled, the behavior is the same.)
Comment on attachment 348685 [details] [review] Patch for the 1.8 branch a=asac for 1.8.0