XPCNativeWrapper pollution using top-level statement of chrome JS

VERIFIED FIXED

Status

()

P2
normal
VERIFIED FIXED
10 years ago
9 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

({testcase, verified1.8.1.19, verified1.9.0.5})

unspecified
testcase, verified1.8.1.19, verified1.9.0.5
Points:
---
Bug Flags:
blocking1.9.1 -
blocking1.9.0.5 +
wanted1.9.0.x +
blocking1.8.1.19 +
wanted1.8.1.x +
blocking1.8.0.next +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [sg:critical])

Attachments

(2 attachments)

(Reporter)

Description

10 years ago
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
(Reporter)

Comment 1

10 years ago
Created attachment 336487 [details]
testcase 1

This works on trunk and fx3.0.x.
(Reporter)

Comment 2

10 years ago
Created attachment 336488 [details]
testcase 2

This works on fx2.0.0.x.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking1.9.1?
Flags: blocking1.9.0.3?
Flags: blocking1.8.1.18?
Keywords: testcase
OS: Windows XP → All
Hardware: PC → All
Blocks: 441087
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.3+
Flags: blocking1.8.1.18?
Flags: blocking1.8.1.18+
Whiteboard: [sg:critical]
Assignee: nobody → mrbkap

Updated

10 years ago
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Not going to block 1.9.1 on this after all, but Blake, please do investigate!
Flags: blocking1.9.1+ → blocking1.9.1-
Created attachment 344195 [details] [diff] [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.
Attachment #344195 - Flags: review?(brendan)

Updated

10 years ago
Attachment #344195 - Flags: superreview+
Attachment #344195 - Flags: review?(brendan)
Attachment #344195 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/b91e44d05452
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [sg:critical] → [sg:critical] needs branch patch
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.
Flags: blocking1.9.0.4?
Flags: blocking1.9.0.4+
Flags: blocking1.8.1.18?
Flags: blocking1.8.1.18+
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!).
Flags: blocking1.9.0.5+
Flags: blocking1.9.0.4?
Flags: blocking1.8.1.19+
Flags: blocking1.8.1.18?
Comment on attachment 344195 [details] [diff] [review]
Proposed fix

This applies as-is to the 1.9 branch.
Attachment #344195 - Flags: approval1.9.0.5?
Comment on attachment 344195 [details] [diff] [review]
Proposed fix

Approved for 1.9.0.5, a=dveditz for release-drivers 

We need a separate 1.8.1.x patch, right?
Attachment #344195 - Flags: approval1.9.0.5? → approval1.9.0.5+
Whiteboard: [sg:critical] needs branch patch → [sg:critical][needs 1.8 patch]
Created attachment 348685 [details] [diff] [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.
Attachment #348685 - Flags: review?(jst)
Attachment #348685 - Flags: approval1.8.1.19?
Comment on attachment 348685 [details] [diff] [review]
Patch for the 1.8 branch

Oops, this is a patch for the 1.8 branch.
Attachment #348685 - Attachment description: Patch for the 1.9 branch → Patch for the 1.8 branch

Updated

10 years ago
Attachment #348685 - Flags: superreview+
Attachment #348685 - Flags: review?(jst)
Attachment #348685 - Flags: review+
Fixed on the 1.9 branch.
Keywords: fixed1.9.0.5
Whiteboard: [sg:critical][needs 1.8 patch] → [sg:critical]
Comment on attachment 348685 [details] [diff] [review]
Patch for the 1.8 branch

Approved for 1.8.1.19, a=dveditz for release-drivers
Attachment #348685 - Flags: approval1.8.1.19? → approval1.8.1.19+
Fixed on the 1.8 branch.
Keywords: fixed1.8.1.19
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.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.19, fixed1.9.0.5 → verified1.8.1.19, verified1.9.0.5

Comment 16

10 years ago
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.
(Reporter)

Comment 17

10 years ago
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 18

10 years ago
Comment on attachment 348685 [details] [diff] [review]
Patch for the 1.8 branch

a=asac for 1.8.0
Attachment #348685 - Flags: approval1.8.0.next+

Updated

10 years ago
Flags: blocking1.8.0.next+
Group: core-security
You need to log in before you can comment on or make changes to this bug.