Security Review of the gaia browser (http://browser.gaiamobile.org)

Risk/Priority Ranking Exercise https://wiki.mozilla.org/Security/RiskRatings Priority: 4 (P2) - Mozilla Initiative Operational: 0 - N/A User: 4 - Critical Privacy: 4 - Critical Engineering: 4 - Critical Reputational: 4 - Critical Priority Score: 64

As discussed with :pauljt I am focusing on SSL and Bookmarks.

I have finished looking at the SSL part in the browser. There was not much to look at because the first release of the browser has extremely minimal (UI) support for SSL. The only thing it really does is look at the connection state and then show a green lock in the location bar. From a code review point of view there is not much to say. From a UI point of view, many things are missing in this release: * It is not possible to see certificate details * When the browser has scrolled and the location bar is not visible anymore, there is no hint about the state of the connection anymore. Maybe a thin green line on top of the screen is an idea? * There is no management of certificates. Can't import your own. * When you open a site with an invalid cert is simply says "The address isn't valid - The URL is not valid and cannot be loaded. Web addresses are usually written like http://www.example.com. Make sure you are using forward slashes." - which is terribly confusing. We need a proper UI there and maybe the same 'i know what im doing' dialog we have on other platforms?

For bookmarks and history I have looked at the relevant code in browser.js and places.js My main focus has been on injection and xss. In general the code looks pretty good and solid. Although innerHTML is used, the content is always escaped properly by first injecting it in a span and then taking the escaped contents of it. I have found three cases that *might* have an issue. In addBookmark() - if (!title) title = uri; Can we mess with the title through the URI? In drawAwesomescreenListItem() there is something going on with url.innerHTML = Utils.createHighlightHTML() I wonder if it is possible to inject a search term in there. I don't think it is because if you search for "<foo>" then no results will be found and no results will be displayed anyway. The modal_dialog code needs to be looked at. In show() in case of a 'custom-prompt' there is no html escaping. But maybe that is intentional because a custom prompt allows any kind of content. I don't think web content can create custom dialogs anyway?

(In reply to Stefan Arentz [:st3fan] from comment #3) > I have finished looking at the SSL part in the browser. There was not much > to look at because the first release of the browser has extremely minimal > (UI) support for SSL. The only thing it really does is look at the > connection state and then show a green lock in the location bar. > > From a code review point of view there is not much to say. > > From a UI point of view, many things are missing in this release: > > * It is not possible to see certificate details > * When the browser has scrolled and the location bar is not visible anymore, > there is no hint about the state of the connection anymore. Maybe a thin > green line on top of the screen is an idea? > * There is no management of certificates. Can't import your own. > * When you open a site with an invalid cert is simply says "The address > isn't valid - The URL is not valid and cannot be loaded. Web addresses are > usually written like http://www.example.com. Make sure you are using forward > slashes." - which is terribly confusing. We need a proper UI there and maybe > the same 'i know what im doing' dialog we have on other platforms? I've argued this case to death, and got beaten back everytime. See bug 796362. I actually looked into it, and I think it would be a simple patch to change the error message being displayed ( I think its just in https://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/netError.xhtml) Maybe we should just submit a patch and see if they will take it. But yes, proper bad certificate UI is a 'feature' that wasnt included in v1.

(In reply to Stefan Arentz [:st3fan] from comment #4) > For bookmarks and history I have looked at the relevant code in browser.js > and places.js > > My main focus has been on injection and xss. > > In general the code looks pretty good and solid. Although innerHTML is used, > the content is always escaped properly by first injecting it in a span and > then taking the escaped contents of it. > > I have found three cases that *might* have an issue. > > In addBookmark() - if (!title) title = uri; Can we mess with the title > through the URI? > > In drawAwesomescreenListItem() there is something going on with > url.innerHTML = Utils.createHighlightHTML() I wonder if it is possible to > inject a search term in there. I don't think it is because if you search for > "<foo>" then no results will be found and no results will be displayed > anyway. > > The modal_dialog code needs to be looked at. In show() in case of a > 'custom-prompt' there is no html escaping. But maybe that is intentional > because a custom prompt allows any kind of content. I don't think web > content can create custom dialogs anyway? Hmm is custom-prompt linked to the way alert/prompt/confirm are displayed? I am pretty sure I have tested this already though. With mozBrowser iframes, when the child content calls something like window.alert, an event is fired (mozbrowsershowmodalprompt) so that browser content can handle this, since there is no XUL to display the dialog. Could we fake this event somehow, so that the custom-prompt code is reached?

No further reviews will be undertaken of the browser.