Last Comment Bug 439034 - Same-origin check in nsXMLDocument::OnChannelRedirect() can be circumvented
: Same-origin check in nsXMLDocument::OnChannelRedirect() can be circumvented
Status: RESOLVED FIXED
[sg:high xss]
: verified1.8.1.17
Product: Core
Classification: Components
Component: Security (show other bugs)
: 1.8 Branch
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-06-13 01:50 PDT by moz_bug_r_a4
Modified: 2008-11-16 20:43 PST (History)
6 users (show)
dveditz: blocking1.8.1.17+
dveditz: wanted1.8.1.x+
asac: blocking1.8.0.next+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
1.8 version of the trunk fix we took for this. (2.92 KB, patch)
2008-08-14 18:16 PDT, Johnny Stenback (:jst, jst@mozilla.com)
jonas: review+
jonas: superreview+
Details | Diff | Review
Updated diff. (4.43 KB, patch)
2008-08-15 14:33 PDT, Johnny Stenback (:jst, jst@mozilla.com)
jst: review+
jst: superreview+
dveditz: approval1.8.1.17+
asac: approval1.8.0.next+
Details | Diff | Review
Trunk version of the cleanup in the above patch. (1.09 KB, patch)
2008-08-15 14:35 PDT, Johnny Stenback (:jst, jst@mozilla.com)
jonas: review+
jonas: superreview+
Details | Diff | Review

Description moz_bug_r_a4 2008-06-13 01:50:24 PDT
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] 2008-07-02 15:52:43 PDT
qawanted: is this fixed by the fix for bug 439035?
Comment 4 Daniel Veditz [:dveditz] 2008-07-02 15:54:17 PDT
-> Johnny if we do need additional fixes
Comment 5 [On PTO until 6/29] 2008-07-02 16:10:55 PDT
(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, jst@mozilla.com) 2008-08-14 18:16:25 PDT
Created attachment 333863 [details] [diff] [review]
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) PTO Until July 5th 2008-08-14 18:19:17 PDT
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, jst@mozilla.com) 2008-08-15 14:33:01 PDT
Created attachment 334011 [details] [diff] [review]
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, jst@mozilla.com) 2008-08-15 14:35:57 PDT
Created attachment 334013 [details] [diff] [review]
Trunk version of the cleanup in the above patch.
Comment 10 Daniel Veditz [:dveditz] 2008-08-15 14:40:33 PDT
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, jst@mozilla.com) 2008-08-15 14:58:52 PDT
Fix landed on the branch, leaving bug open to track the trunk cleanup.
Comment 12 [On PTO until 6/29] 2008-09-02 15:49:11 PDT
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 [On PTO until 6/29] 2008-09-02 15:49:49 PDT
Shouldn't a developer resolve this so I can mark it as verified that way?
Comment 14 Jonas Sicking (:sicking) PTO Until July 5th 2008-09-02 19:48:42 PDT
See comment 11.

Jst: Are you still planning on doing that cleanup?
Comment 15 Johnny Stenback (:jst, jst@mozilla.com) 2008-09-02 21:34:18 PDT
Duh, that cleanup landed two weeks ago, forgot to close the bug...

http://hg.mozilla.org/mozilla-central/rev/61d40733056c
Comment 16 Alexander Sack 2008-09-23 07:54:37 PDT
Comment on attachment 334011 [details] [diff] [review]
Updated diff.

a=asac for 1.8.0.15

Note You need to log in before you can comment on or make changes to this bug.