464174 - The fix in bug 451680 does not fix <field>

Status: RESOLVED FIXED

(Core :: XBL, defect, P1)

Description

[moz_bug_r_a4]

The fix in bug 451680 does not fix <field>.

Comment 1

[moz_bug_r_a4]

Attachment: testcase

This tries to get cookies for www.mozilla.com.
This works on trunk, fx3.0.x and fx2.0.0.x.

Comment 2

[Samuel Sidler (old account; do not CC)]

*sigh*. We probably need to block on this because it affects Firefox 2 and this is our last release there...

Blake? :)

Comment 3

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Proposed fix

This uses the node principal of the bound content's owner document. I *think* that's the right principal to use here.

Comment 4

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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

Using content->NodePrincipal() would be slightly safer I think. Should amount to exactly the same thing.

Comment 5

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Updated to comments

This applies to trunk and the 1.9 branch. I'm looking into backporting it to the 1.8 branch.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

...except that the 1.8 branch isn't vulnerable to this exploit because on the branch, field installation is eager and called from nsXBLProtoImpl::InstallImplementation, which, thanks to the backport in bug 451680, now bails out in this case.

Comment 7

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 349022 [details] [diff] [review]
Updated to comments

After talking to beltzner, we'll wait to check this in after beta2.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Hey, want to remove that XXX comment about a better principal since you have one now?  ;)

Comment 9

[Blake Kaplan (:mrbkap) (inactive)]

Er, yeah. I've done that locally.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 349022 [details] [diff] [review]
Updated to comments

Approved for 1.9.0.5, a=dveditz for release-drivers

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Fixed on the 1.9 branch.

Comment 12

[Johnny Stenback (:jst)]

We took this for 1.9.0, so we can't ship 1.9.1 w/o this. Blocker.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

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'm surprised that we haven't fixed this in Trunk yet though.

Comment 14

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

Comment on attachment 349022 [details] [diff] [review]
Updated to comments

a191=beltzner

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

Note to whoever checks this in -- please use the patch that was actually checked into the 1.9 branch or address comment 8 manually. Checkin message: Bug 464174 - Pass a principal in when compiling fields. r+sr=sicking a=beltzner

Comment 16

[Shawn Wilsher :sdwilsh]

Missed comment 15 before I pushed, so commit message just has bug number and reviewers:
http://hg.mozilla.org/mozilla-central/rev/4cfa752afa85

And addressing comment 8...
http://hg.mozilla.org/mozilla-central/rev/60ba92ead6d3