Security/Reviews/Gaia/Contacts: Difference between revisions

Jump to navigation Jump to search

Security/Reviews/Gaia/Contacts (view source)

Revision as of 16:33, 4 March 2013

4,102 bytes removed ,  4 March 2013
no edit summary
(Created page with "=== App Review Details === * App: Contacts, which is part of the Communications application * Review Date: March 4th, 2013 * Review Lead: Stefan Arentz * Review Bug: {{bug|75473...")
 
No edit summary
Line 87: Line 87:
==== Post Messages ====
==== 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.
ISSUE: None of the handlers verify that the message originated from a trusted/expected source.
Line 111: Line 97:


====Web Activity Usage ====
====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 ====
==== Notable Event Handlers ====
Line 119: Line 103:


====1. XSS & HTML Injection attacks====
====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 ====
====2. Secure Communications ====


===== Remote Services =====
===== 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 =====
===== BlueTooth =====
The dialer blindly accepts phone numbers from a bluetooth device, which are passed directly to <code>mozTelephony</code> without any further (input) validation. Since <code>mozTelephony</code> 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 ====
====3. (Secure) data storage ====
The recent calls database is stored on the device using indexDB. The code for this is abstracted in <code>recents_db.js</code>. Looks pretty solid, no comments.
It also uses asyncStorage, which is implemented in <code>shared/js/async_storage.js</code>. 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 ====
====4. Denial of Service ====
* {{bug|845383}} Dialer accepts super long phone number which breaks the phone until reboot


====5. Use of Privileged APIs ====
====5. Use of Privileged APIs ====


====6. Interfaces with other Apps/Content====
====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 ====
==== 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 ====
==== 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 ===
=== Security Risks & Mitigating Controls ===
Line 176: Line 130:
* {{bug|841071}}  Settings are globally shared between applications
* {{bug|841071}}  Settings are globally shared between applications


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


* {{bug|845945}} Dialer should be turned into minimal standalone application
* {{bug|847417}} Contacts 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)
St3fan
Confirmed users
971

edits

Retrieved from "https://wiki.allizom.org/Special:MobileDiff/535365"

Navigation menu