Security/Reviews/Gaia/Dialer

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to: navigation, search

App Review Details

Overview

This review only looks at the Dialer component of the communications app.

Architecture

Components

The dialer is part of a the communications application. This is an umbrella app that also contains the contacts 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: Dialer should be separated from contacts and facebook

  • bug 845945 Dialer should be turned into minimal standalone application

The dialer has three top level user interfaces:

  • index.html - which shows the keypad
  • incall.html - which shows the ui for incoming and outgoing calls
  • ussd.html - which handles "Unstructured Supplementary Service Data"

Relevant Source Code

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

Application code:

  • dialer/index.html
  • dialer/oncall.html
  • dialer/ussd.html
  • dialer/js/keypad.js
  • dialer/js/dialer.js
  • dialer/js/contacts.js
  • dialer/js/recents.js
  • dialer/js/telephony_helper.js
  • dialer/js/ussd.js

Part of the communications apps:

  • contacts/js/confirm_dialog.js
  • contacts/js/fb/fb_data.js
  • contacts/js/fb/fb_contact_utils.js

Shared code:

  • shared/js/mouse_event_shim.js
  • shared/js/async_storage.js
  • shared/js/l10n.js
  • shared/js/l10n_date.js
  • shared/js/mobile_operator.js
  • shared/js/notification_helper.js
  • shared/js/simple_phone_matcher.js
  • shared/js/settings_listener.js

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

Dial

The dial activity is used to start the process of making a call. 

   "dial": {
     "filters": {
       "type": "webtelephony/number"
     },
     "href": "/dialer/index.html#keyboard-view",
     "disposition": "window"
   }

It is handled at https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L19

The dial handler does not actually dial numbers. The only thing it does is ask the KeypadManager to enter the number. The user will always have to tap the dial button before a call is being made.

ISSUES: The dialer does not correctly validate input. I was able to do multiple malicious things:

  • 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

ACTION: We need better defensive coding around input taken from activities.

System Messages

Since the Dialer is part of the communications app, it accepts a number of 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 dialer installs a handler for the following:

  • notification
  • telephony-new-call
  • bluetooth-dialer-command
  • headset-button
  • ussd-received
notification

The notification message is handled by handleNotification() in https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L52

This message is fired when the user clicks on a missed call notification. The only thing the dialer does is launch itself, to firce itself to the foreground, and open the recents view. No information is taken from the notification or passed to the app.

telephony-new-call

The telephony-new-call message is handled by newCall() in https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L118

This message is fired by ... (TODO Fired by what?) when an incoming call happens. The dialer calls openCallScreen() which loads the oncall.html page in a new window. It uses navigator.mozTelephony to keep track of the call status.

The real work of handling a call is done in https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/oncall.js

bluetooth-dialer-command

The bluetooth-dialer-command message is handler by btCommandHandler in https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L127

This function receives commands from a bluetooth device. I have not found a specification of what kind of commands these devices can send but we handle currently only two:

  • BLDN - To call the most recent number
  • ATD - To start a call. The rest of the command is blindly copied and passed to CallHandler.call()

ISSUE: No input validation is done on commands received from the bluetooth device. I think we should at least make sure that the number is valid

The commands are forwarded to the call screen (oncall.html) with type 'BT'.

TODO: Does this require any interaction on the phone at all? It looks like CallHandler.call() immediately starts making the call. That means if someone can fake/inject a bluetooth message, it is possible to start calls without user interaction. (Stretch, but worth investigating. Related to injecting system messages.)

headset-button

The headset-button message is handled by headsetCommandHandler at https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L152

TODO: This message is received when the button on the headset is pushed?

The message is simply forwarded to the call screen with type 'HS'.

ussd-received

The ussd-received message is handler by UssdManager.openUI() in https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/ussd.js#L137

According to Wikipedia, USSD is ...

<quote> Unstructured Supplementary Service Data (USSD) is a protocol used by GSM cellular telephones to communicate with the service provider's computers. USSD can be used for WAP browsing, prepaid callback service, mobile-money services, location-based content services, menu-based information services, and as part of configuring the phone on the network.</quote>

TODO: This is not easy to understand code in ussd.js .. what messages are we exactly responding to. Will telcos implement this?

Notifications

The app handles notifications at https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/communications/dialer/js/dialer.js#L52

The only thing it does is bring up the dialer in the #recents-view tab.

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)
Retrieved from "https://wiki.mozilla.org/index.php?title=Security/Reviews/Gaia/Dialer&oldid=531602"