moz_bug_r_a4 Reporter
	Description • 16 years ago

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

	moz_bug_r_a4 Reporter
	Comment 1 • 16 years ago

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

	Samuel Sidler (old account; do not CC)
	Comment 2 • 16 years ago

This looks like Blake's bug for the next release.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 3 • 16 years ago

We *want* this badly. But it's going to cause regressions. I'm attaching it here so jst can run mochitests with it.

	Johnny Stenback (:jst)
	Comment 4 • 16 years ago

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.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 5 • 16 years ago

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.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 6 • 16 years ago

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!

	Boris Zbarsky [:bzbarsky]
	Comment 7 • 16 years ago

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.)

	Samuel Sidler (old account; do not CC)
	Comment 8 • 16 years ago

Pushing out per Blake.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 9 • 16 years ago

This appears to work. I did a grep for GetScriptGlobalObject in content/xbl/src/* and these callsites seemed like the ones that wanted patching.

	Boris Zbarsky [:bzbarsky]
	Comment 10 • 16 years ago

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).

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 11 • 16 years ago

http://hg.mozilla.org/mozilla-central/rev/e92dfeb501cd

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 12 • 16 years ago

Comment on attachment 344712 [details] [diff] [review]
Whack harder

This applies cleanly to the 1.9 branch.

	Daniel Veditz [:dveditz]
	Comment 13 • 16 years ago

Comment on attachment 344712 [details] [diff] [review]
Whack harder

Approved for 1.9.0.5, a=dveditz for release-drivers

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 14 • 16 years ago

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.

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 15 • 16 years ago

Checked into the 1.9 branch.

	Daniel Veditz [:dveditz]
	Comment 16 • 16 years ago

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

Approved for 1.8.1.19, a=dveditz for release-drivers

	Blake Kaplan (:mrbkap) (inactive) Assignee
	Comment 17 • 16 years ago

Fixed on the 1.8 branch.

	Al Billings [:abillings - ex-MoCo]
	Comment 18 • 16 years ago

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.

	Alexander Sack
	Comment 19 • 16 years ago

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

a=asac for 1.8.0