125583 - Disable automatic XLinks in Mail

Status: VERIFIED FIXED

(Core :: Security, defect, P2)

Description

[Mitchell Stoltz (not reading bugmail)]

xlink in xml allows opening new windows even with javascript disabled.
While this is not quite big issue, it may be used for advertising purpose and
more importantly it may be used to escape the email sandbox by circumventing
javascript and force the user to visit a page which contain javascript.
The following xml file:
----xlink1.xml------------
<zvon:logo xmlns:zvon = "http://www.zvon.org"
   xmlns:xlink="http://www.w3.org/1999/xlink"
   xlink:type="simple"
   xlink:href="http://www.guninski.com"
   xlink:show="new"
   xlink:actuate="onLoad">
 </zvon:logo>
--------------------------
Shall open www.guninski.com in new window without any interaciotn and even if
javascript is disabled.

Email example:
Put the following in html message:
<iframe src="http://URL_TO_XLINK1/xlink1.xml"></iframe>

When the user previews the message, a new window shall be opened in which
javascript may be enabled.

So if html exploit pops up with this bug it may be exploited by html email
without user interaction.

Comment 1

[Mitchell Stoltz (not reading bugmail)]

Mozilla1.1

Comment 2

[Mitchell Stoltz (not reading bugmail)]

-> heikki

Comment 3

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is related to bug 132105.

I am thinking I could disable automatic XLinks completely in mailnews (without
even giving any prefs). How can I detect if a page is in mailnews window?

Comment 4

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This is strange, nsXMLElement::MaybeTriggerAutoLink() calls CheckLoadURI() with
nsIScriptSecurityManager::DISALLOW_FROM_MAIL, so why does this not stop
automatically loading XLinks in mail? Need to run this through the debugger...

Comment 5

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Ok, so DISALLOW_FROM_MAIL only checks that the link is not allowed to point to
mail protocols.

Mitch pointed out that I could look at
nsScriptSecurityManager::CanExecuteScripts() in order to find out how to detect
if the document is in mailnews window. I looked at it briefly, but one problem
is that the method uses JSContext to get the root docshell, which it uses to
check for apptype. I don't use JSContext at all in nsXMLElement, and I am not
sure how to get the correct one. I do have a webshell though, and I can get a
docshell from there. Maybe I'll be able to get the root docshell from there.
Need to check tomorrow.

Comment 6

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Proposed fix 1

Seems like I can get the docshell and app type by doing QIs etc. from the
webshell. I have disabled automatic XLinks completely in Mail. I don't think we
need a pref at this point, but it will be easy to add if/when it becomes an
issue. I think we would like a pref and more fine grained control if/when
embedding XLinks are implemented (images would work as embedded XLinks, for
example).

I also tweaked a bit how open-during-load pref works for XLinks. Setting it
used to change automatic XLinks that open in new windows into replacing links,
but I think it is better to completely disable those links if the pref is set.

Comment 7

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Changing summary to reflect what the fix will be, and adding Harish and Johnny
to Cc so that they can review.

Comment 8

[harishd]

Comment on attachment 86685 [details] [diff] [review]
Proposed fix 1

r=harishd

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 86685 [details] [diff] [review]
Proposed fix 1

+	   docShellItem->GetRootTreeItem(getter_AddRefs(rootItem));
+	   if (rootItem) {
+	     nsCOMPtr<nsIDocShell> docshell(do_QueryInterface(rootItem));
+	     if (docshell) {

The "if (rootItem)" check is not needed, do_QueryInterface() does that for you.

sr=jst if you remove that redundant check.

Comment 10

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Attachment: Fixed jst's concerns

Comment 11

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Safe fix, can only regress automatically loading XLinks at most. Since JS is
disabled in mail by default this makes sure you cannot go a around that by using
XLinks.

Comment 12

[Jaime Rodriguez, Jr.]

adt1.0.1+ (on ADT's behalf) approval for checkin to the 1.0 branch, pending
Driver's approval. pls check this into the 1.0 branch, then add the "fixed1.0.1"
keyword.

Comment 13

[Johnny Stenback (:jst)]

Approval request sent to [e-mail]

Comment 14

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Checked in on trunk.

Comment 15

[Jaime Rodriguez, Jr.]

resolving as fixed, since this has landed on the trunk. pls add the fixe1.0.1
keyword, once this has been landed on the 1.0 branch. thanks!

bsharma - can you pls verify this fix on the trunk? thanks!

Comment 16

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 86696 [details] [diff] [review]
Fixed jst's concerns

Approved for 1.0 branch.  Please remove mozilla1.0.1+ when it's fixed and add
fixed1.0.1

Comment 17

[Heikki Toivonen (remove -bugzilla when emailing directly)]

Fixed on branch too, but only the "Disable automatic XLinks in Mail" part
because the branch does not have code for making XLinks obey the popup prefs.
Marking fixed.

Comment 18

[bsharma]

Verified on 2002-06-19-trunk build on WinNT

The above test case runs fine on browser and mail client.

Comment 19

[bsharma]

Ran the test cases for the browser and the mail on 2002-06-19-branch build on WinNT.

The test for the browser is passing but the test for the mail is failing.

JS is disabled in both clients.

Reopening the bug.

Comment 20

[Heikki Toivonen (remove -bugzilla when emailing directly)]

This works for me on the branch, how did you test? I sent a test email to
myself, where the mail was in HTML format and the body of the message was
inserted with Insert > HTML... from the menus, and the inserted HTML was:

<iframe src="http://green.nscp.aoltw.net/heikki/125583/xlink1.xml"></iframe>

When I create the message, the automatic XLink fires (so I guess I would also
need to disable it for editor?), but when I receive it, and read it, the XLink
does not work.

There is no browser patr of the test, except to regression test (see that
automatic XLinks still work in browser).

Comment 21

[Heikki Toivonen (remove -bugzilla when emailing directly)]

I just noticed that Bindu tested with 6/19 build, but I only fixed this one the
branch on that day. So I don't think that build had the fix yet. Please verify
with a 6/20 or later branch build. Closing again as fixed.

Comment 22

[bsharma]

Verified on 2002-06-27-trunk and 2002-06-27-branch build on Win2K.

The above test case works as expected in Mail.
Adding verified1.0.1 keyword.