Closed Bug 439034 Opened 11 years ago Closed 11 years ago

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

Categories

(Core :: Security, defect)

1.8 Branch
x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: moz_bug_r_a4, Assigned: jst)

Details

(Keywords: verified1.8.1.17, Whiteboard: [sg:high xss])

Attachments

(2 files, 1 obsolete file)

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.
Flags: blocking1.8.1.16?
Whiteboard: [sg:high xss]
qawanted: is this fixed by the fix for bug 439035?
Keywords: qawanted
-> Johnny if we do need additional fixes
Assignee: nobody → jst
Flags: wanted1.8.1.x+
(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.
Keywords: qawanted
Flags: blocking1.8.1.17? → blocking1.8.1.17+
Whiteboard: [sg:high xss] → [sg:high xss][needs 1.8 patch]
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.
Attachment #333863 - Flags: superreview?(jonas)
Attachment #333863 - Flags: review?(jonas)
Comment on attachment 333863 [details] [diff] [review]
1.8 version of the trunk fix we took for this.

Could shorten to

return nsContentUtils::GetSecurityManager...
Attachment #333863 - Flags: superreview?(jonas)
Attachment #333863 - Flags: superreview+
Attachment #333863 - Flags: review?(jonas)
Attachment #333863 - Flags: review+
Whiteboard: [sg:high xss][needs 1.8 patch] → [sg:high xss]
Attachment #333863 - Flags: approval1.8.1.17?
Attached patch Updated diff.Splinter Review
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.
Attachment #334011 - Flags: superreview+
Attachment #334011 - Flags: review+
Attachment #334011 - Flags: approval1.8.1.17?
Attachment #333863 - Attachment is obsolete: true
Attachment #334013 - Flags: superreview?(jonas)
Attachment #334013 - Flags: review?(jonas)
Attachment #333863 - Flags: approval1.8.1.17?
Comment on attachment 334011 [details] [diff] [review]
Updated diff.

Approved for 1.8.1.17, a=dveditz for release-drivers.
Attachment #334011 - Flags: approval1.8.1.17? → approval1.8.1.17+
Fix landed on the branch, leaving bug open to track the trunk cleanup.
Status: NEW → ASSIGNED
Keywords: fixed1.8.1.17
Attachment #334013 - Flags: superreview?(jonas)
Attachment #334013 - Flags: superreview+
Attachment #334013 - Flags: review?(jonas)
Attachment #334013 - Flags: review+
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.
Shouldn't a developer resolve this so I can mark it as verified that way?
See comment 11.

Jst: Are you still planning on doing that cleanup?
Duh, that cleanup landed two weeks ago, forgot to close the bug...

http://hg.mozilla.org/mozilla-central/rev/61d40733056c
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment on attachment 334011 [details] [diff] [review]
Updated diff.

a=asac for 1.8.0.15
Attachment #334011 - Flags: approval1.8.0.15+
Group: core-security
Flags: blocking1.8.0.15+
You need to log in before you can comment on or make changes to this bug.