Closed Bug 464174 Opened 15 years ago Closed 15 years ago

The fix in bug 451680 does not fix <field>

Categories

(Core :: XBL, defect, P1)

Product:
Core
Component:
XBL
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9.1b3

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

References

Details

(Keywords: fixed1.9.1, verified1.8.1.19, verified1.9.0.5, Whiteboard: [sg:high] fixed in 1.8.1.x by bug 451680)

Attachments

(2 files)

Proposed fix
3.21 KB, patch
sicking
: review+
sicking
: superreview+
Details | Diff | Splinter Review
Updated to comments
3.22 KB, patch
mrbkap
: review+
mrbkap
: superreview+
beltzner
: approval1.9.1+
dveditz
: approval1.9.0.5+
Details | Diff | Splinter Review 
The fix in bug 451680 does not fix <field>.
Attached file testcase — 
This tries to get cookies for www.mozilla.com.
This works on trunk, fx3.0.x and fx2.0.0.x.
*sigh*. We probably need to block on this because it affects Firefox 2 and this is our last release there...

Blake? :)
Assignee: nobody → mrbkap
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.0.5+
Flags: blocking1.8.1.19+
Whiteboard: [sg:high]
Whiteboard: [sg:high] → [sg:high][needs branch patches]
Attached patch Proposed fix — — Splinter Review 
This uses the node principal of the bound content's owner document. I *think* that's the right principal to use here.
Attachment #348911 - Flags: superreview?(jonas)
Attachment #348911 - Flags: review?(jonas)
Whiteboard: [sg:high][needs branch patches] → [sg:high][needs r/sr sicking]
Attachment #348911 - Flags: superreview?(jonas)
Attachment #348911 - Flags: superreview+
Attachment #348911 - Flags: review?(jonas)
Attachment #348911 - Flags: review+
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.
Whiteboard: [sg:high][needs r/sr sicking] → [sg:high][needs branch patches? or just approval?]
This applies to trunk and the 1.9 branch. I'm looking into backporting it to the 1.8 branch.
Attachment #349022 - Flags: superreview+
Attachment #349022 - Flags: review+
Attachment #349022 - Flags: approval1.9.1b2?
Attachment #349022 - Flags: approval1.9.0.5?
...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.
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.19?
Flags: blocking1.8.1.19+
Depends on: CVE-2008-5511
Flags: wanted1.8.1.x?
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.19?
Flags: blocking1.8.1.19+
Keywords: fixed1.8.1.19
Whiteboard: [sg:high][needs branch patches? or just approval?] → [sg:high][needs approval] fixed in 1.8.1.x by bug 451680
Comment on attachment 349022 [details] [diff] [review]
Updated to comments

After talking to beltzner, we'll wait to check this in after beta2.
Attachment #349022 - Flags: approval1.9.1b2? → approval1.9.1?
Component: Security → XBL
Flags: blocking1.9.1?
OS: Windows XP → All
QA Contact: toolkit → xbl
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1
Version: unspecified → Trunk
Hey, want to remove that XXX comment about a better principal since you have one now?  ;)
Er, yeah. I've done that locally.
Comment on attachment 349022 [details] [diff] [review]
Updated to comments

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #349022 - Flags: approval1.9.0.5? → approval1.9.0.5+
Fixed on the 1.9 branch.
Status: NEW → ASSIGNED
Keywords: fixed1.9.0.5
We took this for 1.9.0, so we can't ship 1.9.1 w/o this. Blocker.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P1
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.
Attachment #349022 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 349022 [details] [diff] [review]
Updated to comments

a191=beltzner
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
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
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Target Milestone: mozilla1.9.1 → mozilla1.9.1b3
Group: core-security
Whiteboard: [sg:high][needs approval] fixed in 1.8.1.x by bug 451680 → [sg:high] fixed in 1.8.1.x by bug 451680
You need to log in before you can comment on or make changes to this bug.