Security/Reviews/Gaia/Contacts

< Security‎ | Reviews‎ | Gaia
Revision as of 16:33, 4 March 2013 by St3fan (talk | contribs)

App Review Details

This review also covers the Facebook integration because that is mostly integrated in the Contacts application.

Overview

Architecture

Components

The contacts app is part of a the communications application. This is an umbrella app that also contains the dialer app, the connect-to-facebook app and the first-time-setup app. I say 'app' here, but they are really just pages as part of the single communications app.

This is not great from a security point of view and should ideally be fixed in a future version. The best scenario is that these are three different applications. If they have to talk to eachother they should use WebActivities. This greatly reduces the attack surface.

ISSUE: Contacts should be separated from contacts and maybe even facebook

  • bug 847417 Contacts should be turned into minimal standalone application

Relevant Source Code

Source code can be found at https://github.com/mozilla-b2g/gaia/tree/v1-train/apps/communications/contacts

TODO List relevant files

Permissions

The (communications) app has the following permissions:

*   "telephony":{},
*   "voicemail":{},
*   "contacts":{ "access": "readwrite" },
*   "mobileconnection":{},
*   "attention":{},
*   "settings":{ "access": "readwrite" },
*   "desktop-notification":{},
*   "alarms": {},
*   "systemXHR": {},
*   "wifi-manage":{},
*   "time": {},
*   "audio-channel-telephony":{},
*   "audio-channel-ringer":{},
*   "browser":{}

ISSUE: Because the communications app is an 'umbrella app' for dialer, contacts, first-time setup and some generic facebook code, it has more permissions than it needs to have. Ideally we separate the three applications in the communications app into three separate apps.

Web Activity Handlers

new

TODO

update

TODO

pick

TODO

System Messages

The (communications) app listens to the following system messages:

 "messages": [
    { "alarm": "/facebook/fb_sync.html" },
    { "bluetooth-dialer-command": "/dialer/index.html#keyboard-view" },
    { "headset-button": "/dialer/index.html#keyboard-view" },
    { "notification": "/dialer/index.html#keyboard-view" },
    { "telephony-new-call": "/dialer/index.html#keyboard-view" },
    { "ussd-received": "/dialer/index.html#keyboard-view" }
 ]

The contacts app does not handle any of these messages. They are all for the Dialer and Facebook integration.

alarm

The alarm message is used by the facebook integration code to schedule periodic updates of the Facebook contact data.

The code for this can be found in https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/facebook/js/fb_sync_init.js#L22

Post Messages

ISSUE: None of the handlers verify that the message originated from a trusted/expected source.

I was able to exploit this and let remote content post messages to the Dialer to trigger Missed Calls notifications to appear.

ACTION: Add strict checking of event sources as described on MDN at https://developer.mozilla.org/en-US/docs/DOM/window.postMessage#Security_concerns

  • bug 845487 Dialer responds to cross-origin messages without verifying the source (exploitable)

Web Activity Usage

Notable Event Handlers

Code Review Notes

1. XSS & HTML Injection attacks

2. Secure Communications

Remote Services
BlueTooth

3. (Secure) data storage

4. Denial of Service

5. Use of Privileged APIs

6. Interfaces with other Apps/Content

7. Oddities

8. Input Validation Problems

Security Risks & Mitigating Controls

Actions & Recommendations

The dialer unnecessarily has access to all system settings. This is an issue with the Settings API that should be improved in a future version of Firefox OS:

  • bug 841071 Settings are globally shared between applications

The contacts app is embedded in a bigger app, which is not great from a security pov:

  • bug 847417 Contacts should be turned into minimal standalone application