Closed Bug 451680 (CVE-2008-5511) Opened 12 years ago Closed 12 years ago

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

Categories

(Core :: Security, defect, P1)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED

People

(Reporter: moz_bug_r_a4, Assigned: mrbkap)

References

Details

(Keywords: verified1.8.1.19, verified1.9.0.5, Whiteboard: [sg:high])

Attachments

(3 files, 2 obsolete files)

It's possible to use a binding to perform an XSS attack in a similar way to bug
428672 and bug 433328.
Attached 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]
Attached 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)
Attached 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+
Attached 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: 12 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
Attachment #348669 - Flags: approval1.8.0.next+
Comment on attachment 348669 [details] [diff] [review]
Patch for the 1.8 branch

a=asac for 1.8.0
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.