Security/Reviews/Gaia/sms/jun13

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

App Review Details

  • App: SMS
  • Review Date: 4th June 2013
  • Review Lead: Paul Theriault

Overview

SMS app provides the ability to send and recieve SMS messages.


Components

SMS app is structed as a single HTML page. This page displays the list of SMS messages, UI for composing sms, viewing threads, deleting messages etc. There is also web activity handlers so that other apps can ask the SMS app to send an SMS.

Inputs to the SMS app: SMS events from system, web activity requests, data from contacts, user input

Note that navigation in the app is performed in some cases via modify

Relevant Source Code

Source code is viewable here: http://mxr.mozilla.org/gaia/source/apps/sms/js/ This review based on the snapshot of the v1-train branch: v1-train 055ff9c Merge pull request #8773 from davidflanagan/bug847060 v1.0.1 was also examined during this review.

Permissions

The sms app reuires the following permisisons:

 "permissions": {
   "sms":{},
   "contacts":{ "access": "readonly" },
   "mobileconnection":{},
   "settings":{ "access": "readonly" },
   "audio-channel-notification":{},
   "desktop-notification":{}
 },

The SMS app does not currently appear to use mobileconnection API. This should be removed if not in use.

Web Activity Handlers

The SMS app handles one activity:

 "activities": {
   "new": {
     "filters": {
       "type": "websms/sms",
       "number": { "regexp":"/^[\\w\\s+#*().-]{0,50}$/" }
      },
     "disposition": "window"
   }
 }

Web Activity Usage

The SMS initiates outgoing activities for links. A pick activity is also used to retrieve a contact from the address book (or any app that provides the right activity).


Notable Event Handlers

window.addEventListener('hashchange'

If you could cause the sms app to navigate somehow, then this might be an issue, since hashchange causes this to happen in the UI. But you can't frame or link to app:// so I dont think this is an issue.


Code Review Notes

1. XSS & HTML Injection attacks

InnerHTML used dangerously despite presence of templating system e.g: http://mxr.mozilla.org/gaia/source/apps/sms/js/waiting_screen.js#22 (doesnt seem to be called though)


2. Secure Communications

N/A Nothing loaded remotely

3. Secure data storage

Sms data is stored in indexeddb, which is stored inside the profile, which is to be protected via file permissions.

4. Denial of Service

No issues identified.

5. Use of Privileged APIs

The app uses privileged APIs in a sane manner - can't see any opportunity for hardening.

6. Interfaces with other Apps/Content

No issues other than web activity issues previously raised.


Actions & Recommendations

  • bug 879136
  • Remove mobileconnection permission since it isn't used.