902381 - Email app should not open URLs using window.open

Status: RESOLVED INVALID

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Frederik Braun [:freddy]]

The code that is invoked when clicking on a hyperlink is as follows (in function 
onHyperlinkClick):
./js/cards/message_reader.js:524:          window.open(linkUrl, '_blank');


This will open the URL *within* te email app, allowing potentially malicious URLs (like data URIs that share the same origin with the opener) access the whole DOM, the JS scope and execute arbitrary JavaScript within the email app.

Of course our HTML sanitizer takes care of data URIs and the like (does it? I will test..), but as a defense-in-depth mechanism we shouldn't really open it in the email app:

Aa WebActivity should be fired that opens the URL in the borwser.

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

:jlebar, your input is appreciated here on the platform issues. :jsicking, your webapi thoughts.

The rationale behind our use of window.open() was that it was more webby than our still-proprietary MozActivity implementations and '_blank' has explicit web semantics that would do what we wanted in a desktop browser too.  I forget if I made that decision on my own or asked someone; I don't have an obvious e-mail trail.  I know I primarily audited the system app path and the platform details I audited have likely changed, so there could be dangers related to potentially magic schemes like javascript (although those should be prevented by our sanitizer which should only allow http/https through right now and CSP).

My understanding of how this works is:

- a "mozbrowseropenwindow" event gets fired. https://developer.mozilla.org/en-US/docs/Web/Reference/Events/mozbrowseropenwindow

- the system app's popup manager sees our event and that we asked for "_blank" and converts our request into a 'view' activity. (The window_manager ignores the event because we're not talking about the 'remote' magic flag.)  If we hadn't asked for _blank, PopupManager.open() would trigger and load the URL in an iframe for us.  https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/popup_manager.js#L165


I don't have a problem with switching to directly opening a web activity since that's really the intent of what we're doing right now, but it seems like we want to make window.open() safe for either everyone or privileged/certified apps if it is a risk.  I do think we probably want to get https://developer.mozilla.org/en-US/docs/Web/Apps/Security_guidelines updated so it says something about window.open and https://developer.mozilla.org/en-US/docs/Web/Reference/Events/mozbrowseropenwindow so that it explains whether all window.open() calls will generate an event or if 'javascript:' would not, etc.

Also, it's not clear to me whether window.open would actually be needed for the new UX paradigm Josh and team demoed the other week?  I think we would definitely want the e-mail app in the URL's "back" trail.  If an activity gets us that, that works.

Comment 2

[Justin Lebar (not reading bugmail)]

Based on comment 1, it sounds like window.open(_blank) is the right thing to do.  If it's not working properly (e.g. if it's opening a frame within the e-mail app), then Gaia isn't doing the right thing.

Comment 3

[Frederik Braun [:freddy]]

A misunderstanding on my side. Thanks for the clarification, Justin and Andrew :)