304690 - Stand-alone applications can do cross-domain scripting through Firefox

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Michael Krax]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.10) Gecko/20050716 Firefox/1.0.6

As far as i understand the fix for bug #298255 it was intendet to stop the
ability to load chrome urls from external applications and to clear the context
of javascript calls to about:blank.

Tested with Firefox 1.0.6 and it works as expected: can not load chrome and
javascript urls don't have access to document.cookie (although a
javascript:alert(document.cookie) gives the current domain as title of the alert
dialog instead of "javascript application" or "about:blank").

But by calling a data: url it is still possible to do cross-domain scripting. Is
this expected behavior in 1.0.6 or was this overlooked?

Reproducible: Always

Comment 1

[Michael Krax]

Attachment: testcase

Comment 2

[Michael Krax]

Attachment: testcase mov file

Comment 3

[Jesse Ruderman]

Mac OS X 10.4.2
QuickTime Version 7.0.1, Player Version 7.0.1 (7.0.1)
Mozilla/5.0 (Macintosh; U; PPC Mac OS X Mach-O; en-US; rv:1.8b4) Gecko/20050816
Firefox/1.0+

Clicking the "Clicking here" in the .mov file doesn't seem to have any effect on
Firefox.  I'm guessing this testcase was made for Windows, so I'll test on
Windows later.

Comment 4

[Mike Connor [:mconnor]]

XP SP2 and QT 6.5.2 here, and I get nothing.  I do get the alert(1) in the first
testcase, but that's just an onclick for the element.

What combination of OS/QT is vulnerable to this?

Comment 5

[Michael Krax]

Attachment: new testcase

Sorry, the first testcase was missing a file. Uploaded a new one as a complete
zip (should contain three files). This is proven to work with QT 6.5.1 on
Windows XP pro SP2 with latest patches and Firefox 1.0.6 as default browser. It
uses a documented QT feature - so it should also work on OSX.

Comment 6

[Jesse Ruderman]

I tried the new testcase on Windows XP.  QuickTime tried to open the data: URL
in IE7 even though Firefox is my default browser, so I wasn't really able to
test the exploit.  I have QuickTime 7.0.2a63.

Comment 7

[Michael Krax]

Tested with latest production release of QT (6.5.2) and Firefox 1.0.6 on WinXP
SP1 with IE6, WinXP SP2 with IE7 and on Vista Beta 1 (IE7). All working. 

The upgrade to the QT 7 preview failed on Vista (probably because of 64bit) and
i can't upgrade the XP machines right now. 

Jesse, can you try to downgrade to QT 6.5.2?

Comment 8

[Jesse Ruderman]

Same thing happens with QT 6.5.2.

Comment 9

[Jesse Ruderman]

I figured out how to reproduce this bug using Start->Run.  I think that for this
exploit to work, the option for "Open links from other applications in" must be
"the most recent tab/window".

"C:\moz\firefox 0818 branch\firefox.exe" "chrome://browser/content/browser.xul"
* Does nothing if Firefox is already loaded.
* If Firefox is not already loaded, incorrectly loads browser.xul into the
content area. (#1)

Navigate to http://www.google.com/ and then:
"C:\moz\firefox 0818 branch\firefox.exe" "javascript:alert(document.cookie)"
* Does not show the cookie.
* The title bar is blank when the dialog appears.
* Shows above content from www.google.com. (#2)
* Incorrectly shows "www.google.com" in the title of the dialog. (#3)
* The back button doesn't do what I'd expect. (#4)

Navigate to http://www.google.com/ and then:
"C:\moz\firefox 0818 branch\firefox.exe"
"data:text/html,<script>alert(document.cookie)</script>"
* Shows the cookie. (#5 - this bug)

Comment 10

[Jesse Ruderman]

I filed bug 305269 for #1.  Of the remaining issues, I don't know how many of
them are the same bug.

Comment 11

[Jesse Ruderman]

I can reproduce on Mac using the following AppleScript script:

tell application "DeerPark"
	activate
	Get URL "data:text/html,<script>alert(document.cookie)</script>"
end tell

Comment 12

[Jesse Ruderman]

#2 in comment 9 might be bug 253951.

Comment 13

[Jesse Ruderman]

Why is the CreateAboutBlankContentViewer() call at
http://lxr.mozilla.org/mozilla/source/docshell/base/nsDocShell.cpp#6203 not
preventing data: URLs from looking at the previous page's cookies?  (jst/caillon?)

Comment 14

[Mike Connor [:mconnor]]

-> dveditz, since he's probably the best person to answer comment 13 here, and
to fix this.

Comment 15

[Asa Dotzler [:asa]]

we'd like to get a fix before the rc.

Comment 16

[:Gavin Sharp [email: [e-mail]]]

This is slightly less of a problem now that we've removed the Firefox UI for "in the same tab/window" option, but it would be good to answer the question in comment 13 - I've verified that we are reaching that block of code, but the external data: URI load is still able to get at the previously loaded page's cookies. Is CreateAboutBlankContentViewer(nsnull) not meant to prevent that?

Comment 17

[Boris Zbarsky [:bzbarsky]]

The problem is that this block:

    // Get an owner from the current document if necessary.  Note that we only
    // do this for URIs that inherit a security context; in particular we do
...


comes way before we create the blank content viewer.

Is there ever a case when LOAD_NORMAL_EXTERNAL is the load type _and_ we have a window target?  If not, I would say we should do the LOAD_NORMAL_EXTERNAL check a lot earlier: before we do the window retargeting.

Alternately, we could reset the owner in the LOAD_NORMAL_EXTERNAL block if we inherited it.... that might be easiest.

Comment 18

[Boris Zbarsky [:bzbarsky]]

Attachment: Trunk fix

Does this work?

If so, I'll request module owner reviews etc....

Comment 19

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 261998 [details] [diff] [review]
Trunk fix

Yeah, this fixes it for me.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: Better trunk fix

Comment 21

[Boris Zbarsky [:bzbarsky]]

Attachment: Branch fix

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 262305 [details] [diff] [review]
Better trunk fix

r=dveditz

Comment 23

[Daniel Veditz [:dveditz]]

Comment on attachment 262312 [details] [diff] [review]
Branch fix

>+    if (aLoadType == LOAD_NORMAL_EXTERNAL [...]

Shouldn't this be != like the trunk patch and comment?

Comment 24

[Boris Zbarsky [:bzbarsky]]

Attachment: Correct branch fix

> Shouldn't this be != like the trunk patch and comment?

Yes, it should...

Comment 25

[Daniel Veditz [:dveditz]]

Comment on attachment 262601 [details] [diff] [review]
Correct branch fix

r=dveditz

Comment 26

[Boris Zbarsky [:bzbarsky]]

Fixed on trunk.

Comment 27

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 262601 [details] [diff] [review]
Correct branch fix

If it's not too late, we might want this for the next branch releases.  I'm pretty confident that this is a safe patch...

If we're worried that it'll break something odd, then let's do it in 1.8.0.13/1.8.1.5

Comment 28

[Daniel Veditz [:dveditz]]

Comment on attachment 262601 [details] [diff] [review]
Correct branch fix

approved for 1.8.0.12 and 1.8.1.4, a=dveditz

Would love to have this safe fix.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Checked in on branches.

Comment 30

[Jay Patel [:jay]]

v.fixed on 1.8 branch with Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.4) Gecko/20070515 Firefox/2.0.0.4 (Jesse's AppleScript testcase passes for me, I just see the JavaScript Application alert)