App Review Details
- App: Contacts, which is part of the Communications application
- Review Date: March 4th, 2013
- Review Lead: Stefan Arentz
- Review Bug: bug 754738 [Security Review] B2G Gaia - Communications / Contacts
- Dependency Tree: https://bugzilla.mozilla.org/showdependencytree.cgi?id=754738&hide_resolved=0
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
The dialer (and the other code in the communications app) depends on window.postMessage() and setEventHandler('message',...) to send and receive cross origin messages. Usually between different pages in the same app, like dialer/index.html and dialer/oncall.html but also for remote sites like for example when we integrate with Facebook.
The app handles the following post messages:
- js/dialer.js:200
- "closing"
- "notification"
- "recent"
- "contactsiframe"
- js/oncall.js:505
- "exitCallScreen"
- js/ussd.js:33
- "reply"
- "close"
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
The only WebActivity that the dialer uses is in they keypad where it sends out a "new" activity to create a "webcontacts/contact" for the current phone number. This is triggered by hitting the + button next to the dial button. It simply opens the contacts app in the Add Contact screen with the specified phone number filled in.
Notable Event Handlers
Code Review Notes
1. XSS & HTML Injection attacks
None found. The dialer app has no text fields. The keypad only accepts user input through a limited on screen keyboard.
2. Secure Communications
Remote Services
The dialer does not directly talk to remote services. There is talk to Facebook through the Contacts but that will be looked at in the Contacts review.
BlueTooth
The dialer blindly accepts phone numbers from a bluetooth device, which are passed directly to mozTelephony without any further (input) validation. Since mozTelephony passes this data on to the RIL without validation, it opens up possibilities for RIL attacks.
- bug 845930 Dialer does not validate phone numbers received via BlueTooth
3. (Secure) data storage
The recent calls database is stored on the device using indexDB. The code for this is abstracted in recents_db.js. Looks pretty solid, no comments.
It also uses asyncStorage, which is implemented in shared/js/async_storage.js. The app stores the Facebook oauth token in there (the contacts app does, but it is checked in the dialer to see if facebook has been 'connected') and the time the call log was last visited.
There is nothing wrong with this approach, except that because the dialer is part of the communications 'umbrella' app, more code than needed has access to these databases. See:
- bug 845945 Dialer should be turned into minimal standalone application
4. Denial of Service
- bug 845383 Dialer accepts super long phone number which breaks the phone until reboot
5. Use of Privileged APIs
6. Interfaces with other Apps/Content
Because the dialer is part of the communications app, things are complicated and less secure than they can be. The attack described in bug 845487 is only possible because the dialer opens the contacts which opens a remote site, which really all happens in the same application.
It is highly recommended to split up the communications app into separate applications that talk to eachother through WebActivities. This minimizes the attack surface and also decreases the complexity of things having to work together. This results in easier to understand code and smaller attack surfaces.
7. Oddities
The whole USSD code is a bit of a mystery and hard to play with for testing. It is difficult to find out what this code does in practice and how it can be attacked from the outside.
8. Input Validation Problems
- bug 845361 Dialer does not correctly validate input to the dial activity handler
- bug 845383 Dialer accepts super long phone number which breaks the phone until reboot
- bug 845045 Dialer can be tricked into displaying one number but dialing another
Note that all these issues are about phone numbers accepted through the dial activity. In general there is not enough / not strong enough checking of incoming data through activities.
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 dialer is embedded in a bigger app, which is not great from a security pov:
- bug 845945 Dialer should be turned into minimal standalone application
Multiple input validation issues that need to be fixed:
- bug 845383 Dialer accepts super long phone number which breaks the phone until reboot
- bug 845361 Dialer does not correctly validate input to the dial activity handler
- bug 845045 Dialer can be tricked into displaying one number but dialing another
- bug 845930 Dialer does not validate phone numbers received via BlueTooth
The dialer does not verify the source of 'postMessage()' messages:
- bug 845487 Dialer responds to cross-origin messages without verifying the source (exploitable)