canmove, Confirmed users
1,220
edits
Security/Reviews/Gaia/sms: Difference between revisions
no edit summary
Ptheriault (talk | contribs) |
Ptheriault (talk | contribs) No edit summary |
||
| Line 1: | Line 1: | ||
=== App Review Details === | === App Review Details === | ||
* App: SMS | * App: SMS | ||
* Review Date: | * Review Date: 4th June 2013 | ||
* Review Lead: Paul Theriault | * Review Lead: Paul Theriault | ||
| Line 7: | Line 7: | ||
SMS app provides the ability to send and recieve SMS messages. | SMS app provides the ability to send and recieve SMS messages. | ||
====Components==== | ====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==== | ====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 ==== | ====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 ==== | ====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 ==== | ==== 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. | |||
| Line 32: | Line 61: | ||
====1. XSS & HTML Injection attacks==== | ====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 ==== | ====2. Secure Communications ==== | ||
N/A Nothing loaded remotely | |||
====3. Secure data storage ==== | ====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 ==== | ====4. Denial of Service ==== | ||
No issues identified. | |||
====5. Use of Privileged APIs ==== | ====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==== | ====6. Interfaces with other Apps/Content==== | ||
No issues other than web activity issues previously raised. | |||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
Private issue: bug 879136 | |||
Remove mobileconnection permission since it isn't used. | |||
[[Category:SecReview]] | [[Category:SecReview]] | ||