439034 - Same-origin check in nsXMLDocument::OnChannelRedirect() can be circumvented

Status: RESOLVED FIXED

(Core :: Security, defect)

Description

[moz_bug_r_a4]

This is fx2-only.  (On trunk, the same-origin check in question compares an old
URI to a new URI.)

The same-origin check in nsXMLDocument::OnChannelRedirect() uses a principal of
an associated JS context.  It can be circumvented by loading a cross-origin
page in that context before nsXMLDocument::OnChannelRedirect() is called.

By using this trick, it's possible to change a principal of an XML document,
and thus, an attacker can perform an XSS attack.

Upcoming testcase consists of an html and a cgi script, thus it does not work
on bugzilla.mozilla.org.  Please set up it in a suitable place.

Comment 3

[Daniel Veditz [:dveditz]]

qawanted: is this fixed by the fix for bug 439035?

Comment 4

[Daniel Veditz [:dveditz]]

-> Johnny if we do need additional fixes

Comment 5

[Al Billings [:abillings - ex-MoCo]]

(In reply to comment #3)
> qawanted: is this fixed by the fix for bug 439035?
> 

It is not. I checked this with 2.0.0.16 when I verified bug 439035 and it still reproduces even though that one is fixed.

Comment 6

[Johnny Stenback (:jst)]

Attachment: 1.8 version of the trunk fix we took for this.

This fixes this bug by making the same change on the 1.8 branch that we took on the trunk some time back. The scary part here, if any, is the fact that we're removing the ability to do cross site loads with document.load() if the caller has elevated privileges. If need be we could leave that change out, but we haven't heard anything bad about that in Firefox 3, so I wouldn't expect any problems from it.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 333863 [details] [diff] [review]
1.8 version of the trunk fix we took for this.

Could shorten to

return nsContentUtils::GetSecurityManager...

Comment 8

[Johnny Stenback (:jst)]

Attachment: Updated diff.

This fixes the silly return code, and fixes the refcounting problem pointed out to me by dveditz (which was a result of me copying this code from an XPCOMGC tree, duh). Also found some dead code I took out too. This one's good to go.

Comment 9

[Johnny Stenback (:jst)]

Attachment: Trunk version of the cleanup in the above patch.

Comment 10

[Daniel Veditz [:dveditz]]

Comment on attachment 334011 [details] [diff] [review]
Updated diff.

Approved for 1.8.1.17, a=dveditz for release-drivers.

Comment 11

[Johnny Stenback (:jst)]

Fix landed on the branch, leaving bug open to track the trunk cleanup.

Comment 12

[Al Billings [:abillings - ex-MoCo]]

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.17) Gecko/2008082909 Firefox/2.0.0.17.

Comment 13

[Al Billings [:abillings - ex-MoCo]]

Shouldn't a developer resolve this so I can mark it as verified that way?

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

See comment 11.

Jst: Are you still planning on doing that cleanup?

Comment 15

[Johnny Stenback (:jst)]

Duh, that cleanup landed two weeks ago, forgot to close the bug...

http://hg.mozilla.org/mozilla-central/rev/61d40733056c

Comment 16

[Alexander Sack]

Comment on attachment 334011 [details] [diff] [review]
Updated diff.

a=asac for 1.8.0.15