Security/Reviews/Gaia/sms/jun13
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.