Bug 451680 (CVE-2008-5511)

XSS by attaching a binding to an element in an unloaded document

VERIFIED FIXED

Status

()

defect
P1
normal
VERIFIED FIXED
11 years ago
11 years ago

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

Tracking

({verified1.8.1.19, verified1.9.0.5})

unspecified
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 -
wanted1.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:high])

Attachments

(3 attachments, 2 obsolete attachments)

Reporter

Description

11 years ago
It's possible to use a binding to perform an XSS attack in a similar way to bug
428672 and bug 433328.
Reporter

Comment 1

11 years ago
Posted file testcase
This tries to get cookies for www.mozilla.com.
This works on trunk, fx3.0.x and fx2.0.0.x.
This looks like Blake's bug for the next release.
Assignee: nobody → mrbkap
Flags: wanted1.9.0.x+
Flags: wanted1.8.1.x+
Flags: blocking1.9.1?
Flags: blocking1.9.0.3?
Flags: blocking1.8.1.18?
Whiteboard: [sg:high]
Posted patch Not ready for prime time (obsolete) — Splinter Review
We *want* this badly. But it's going to cause regressions. I'm attaching it here so jst can run mochitests with it.
Attachment #335109 - Flags: review?(jst)
Flags: blocking1.9.0.3?
Flags: blocking1.9.0.3+
Flags: blocking1.8.1.18?
Flags: blocking1.8.1.18+
Not blocking the release, but it'd be awesome to at least wall-paper over this for 3.1, but the real fix will have to wait for next release.
Flags: wanted1.9.1+
Flags: blocking1.9.1?
Flags: blocking1.9.1-
Priority: -- → P1
Comment on attachment 335109 [details] [diff] [review]
Not ready for prime time

This caused mochitests to not run at all and this isn't the right bug for this patch. Wallpaper coming right up.
Attachment #335109 - Attachment is obsolete: true
Attachment #335109 - Flags: review?(jst)
Posted patch Whack (obsolete) — Splinter Review
The actual offending call to GetScriptGlobalObject that is biting us is buried deep in nsXBLProtoImpl but rather than play whack-a-mole in there too, I say cut this path off at the head. I don't think it's really valid to call addBinding on a document after it's been torn down anyway. Tell me if I'm wrong!
Attachment #344184 - Flags: superreview?(bzbarsky)
Attachment #344184 - Flags: review?(bzbarsky)
Isn't the right check on the content's ownerDocument?  Otherwise if I take content from doc A and call addBinding() on doc B, won't we do the wrong things (assume A and B same-origin so we pass that check in addBinding)?

Or is the GetScriptGlobalObject happening on some document that's not the ownerDocument of the bound node?

And can we also whack that mole by using GetScopeObject or some such?

(On a side note, I really wish we could just remove addBinding.  It's poorly-tested and all.)
Pushing out per Blake.
Flags: blocking1.9.0.5+
Flags: blocking1.9.0.4+
Flags: blocking1.8.1.19+
Flags: blocking1.8.1.18+
Posted patch Whack harderSplinter Review
This appears to work. I did a grep for GetScriptGlobalObject in content/xbl/src/* and these callsites seemed like the ones that wanted patching.
Attachment #344184 - Attachment is obsolete: true
Attachment #344712 - Flags: superreview?(bzbarsky)
Attachment #344712 - Flags: review?(bzbarsky)
Attachment #344184 - Flags: superreview?(bzbarsky)
Attachment #344184 - Flags: review?(bzbarsky)
Comment on attachment 344712 [details] [diff] [review]
Whack harder

Looks good, but I'm also happy adding bullet-proofing to addBinding if we want (in addition to this change).
Attachment #344712 - Flags: superreview?(bzbarsky)
Attachment #344712 - Flags: superreview+
Attachment #344712 - Flags: review?(bzbarsky)
Attachment #344712 - Flags: review+
http://hg.mozilla.org/mozilla-central/rev/e92dfeb501cd
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [sg:high] → [sg:high][needs 1.8/1.9 patches]
Comment on attachment 344712 [details] [diff] [review]
Whack harder

This applies cleanly to the 1.9 branch.
Attachment #344712 - Flags: approval1.9.0.5?
Whiteboard: [sg:high][needs 1.8/1.9 patches] → [sg:high][needs 1.8 patch]
Comment on attachment 344712 [details] [diff] [review]
Whack harder

Approved for 1.9.0.5, a=dveditz for release-drivers
Attachment #344712 - Flags: approval1.9.0.5? → approval1.9.0.5+
The lack of GetScopeObject on the 1.8 branch made this more complicated. However, in writing this patch, I realized that I really need to hold *strong* refs to the return value of nsIDocument::GetScopeObject (since it's implemented as a weak reference underneath the hood). I'll file a new bug on that, though.
Attachment #348669 - Flags: review?(jst)
Attachment #348669 - Flags: approval1.8.1.19?
Attachment #348669 - Flags: superreview+
Attachment #348669 - Flags: review?(jst)
Attachment #348669 - Flags: review+
Checked into the 1.9 branch.
Keywords: fixed1.9.0.5
Whiteboard: [sg:high][needs 1.8 patch] → [sg:high]
Comment on attachment 348669 [details] [diff] [review]
Patch for the 1.8 branch

Approved for 1.8.1.19, a=dveditz for release-drivers
Attachment #348669 - Flags: approval1.8.1.19? → approval1.8.1.19+
Fixed on the 1.8 branch.
Keywords: fixed1.8.1.19
Blocks: 464174
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 also checked this in trunk so I'm marking it as verified.
Status: RESOLVED → VERIFIED

Updated

11 years ago
Attachment #348669 - Flags: approval1.8.0.next+

Comment 19

11 years ago
Comment on attachment 348669 [details] [diff] [review]
Patch for the 1.8 branch

a=asac for 1.8.0

Updated

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