Last Comment Bug 464174 - The fix in bug 451680 does not fix <field>
: The fix in bug 451680 does not fix <field>
Status: RESOLVED FIXED
: [sg:high] fixed in 1.8.1.x by bug 451680
: fixed1.9.1, verified1.8.1.19, verified1.9.0.5
Product: Core
Classification: Components
Component: XBL
: Trunk
: All All
: P1 normal (vote)
: mozilla1.9.1b3
Assigned To: Blake Kaplan (:mrbkap)
: xbl
:
: CVE-2008-5511
:
  Show dependency treegraph
 
Reported: 2008-11-10 20:50 PST by moz_bug_r_a4
Modified: 2009-01-07 09:32 PST (History)
10 users (show)
jst: blocking1.9.1+
samuel.sidler+old: blocking1.9.0.5+
samuel.sidler+old: wanted1.9.0.x+
dveditz: blocking1.8.1.19+
dveditz: wanted1.8.1.x+
See Also:
Crash Signature:


Attachments
Proposed fix (3.21 KB, patch)
2008-11-18 20:23 PST, Blake Kaplan (:mrbkap)
jonas: review+
jonas: superreview+
Details | Diff | Splinter Review
Updated to comments (3.22 KB, patch)
2008-11-19 12:21 PST, Blake Kaplan (:mrbkap)
mrbkap: review+
mrbkap: superreview+
mbeltzner: approval1.9.1+
dveditz: approval1.9.0.5+
Details | Diff | Splinter Review

Summon comment box

Description moz_bug_r_a4 2008-11-10 20:50:36 PST
The fix in bug 451680 does not fix <field>.
Comment 1 moz_bug_r_a4 2008-11-10 20:52:29 PST
Created attachment 347452 [details]
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) 2008-11-10 21:00:05 PST
*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) 2008-11-18 20:23:50 PST
Created attachment 348911 [details] [review]
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) 2008-11-18 22:51:39 PST
Comment on attachment 348911 [details] [review]
Proposed fix

Using content->NodePrincipal() would be slightly safer I think. Should amount to exactly the same thing.
Comment 5 Blake Kaplan (:mrbkap) 2008-11-19 12:21:16 PST
Created attachment 349022 [details] [review]
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) 2008-11-19 12:40:06 PST
...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) 2008-11-19 13:02:15 PST
Comment on attachment 349022 [details] [review]
Updated to comments

After talking to beltzner, we'll wait to check this in after beta2.
Comment 8 Boris Zbarsky (:bz) 2008-11-19 13:09:03 PST
Hey, want to remove that XXX comment about a better principal since you have one now?  ;)
Comment 9 Blake Kaplan (:mrbkap) 2008-11-19 14:35:50 PST
Er, yeah. I've done that locally.
Comment 10 Daniel Veditz 2008-11-19 15:34:41 PST
Comment on attachment 349022 [details] [review]
Updated to comments

Approved for 1.9.0.5, a=dveditz for release-drivers
Comment 11 Blake Kaplan (:mrbkap) 2008-11-19 16:04:31 PST
Fixed on the 1.9 branch.
Comment 12 Johnny Stenback (:jst, jst@mozilla.com) 2008-11-25 14:45:06 PST
We took this for 1.9.0, so we can't ship 1.9.1 w/o this. Blocker.
Comment 13 Al Billings [:abillings] 2008-11-25 17:23:01 PST
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] 2008-11-26 13:32:18 PST
Comment on attachment 349022 [details] [review]
Updated to comments

a191=beltzner
Comment 15 Blake Kaplan (:mrbkap) 2008-11-28 18:10:53 PST
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 2008-11-28 18:26:21 PST
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

Note You need to log in before you can comment on or make changes to this bug.