Security/Reviews/Gaia/sms: Difference between revisions
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: 27th August 2013 | ||
* Review Lead: | * Latest Commit: https://github.com/mozilla-b2g/gaia/commit/b4ca01e782c5d03ff4af870625dd2ebcb4ecb17e | ||
* Branch Reviewed : Master | |||
* Review Lead: Stéphanie Ouillon | |||
=== Overview === | === Overview === | ||
SMS app provides the ability to send and | SMS app provides the ability to send and receive SMS/MMS messages. SMS was reviewed previously in jun 2013. This review has been completed primarily to include the addition of MMS functionality. The prior review can be found [https://wiki.mozilla.org/Security/Reviews/Gaia/sms/jun13 here] | ||
===Architecture=== | |||
====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. | |||
A message is composed of a phone number and optional body, contact and threadId. | |||
There is also web activity handlers so that other apps can ask the SMS app to send a message. | |||
Inputs to the SMS app: SMS events from system, web activity requests, data from contacts, user input to write the SMS. | |||
At startup, 4 components are initialized: | |||
* ActivityHandler: application's global Activity handler to delegate to specific handler based on the Activity name; action when receiving a sms/mms. | |||
* ThreadUI: UI actions (listeners, view initialization, message templating...) | |||
* MessageManager: message actions (send, receive...) | |||
* Settings: set the default mms message size limitation | |||
====Relevant Source Code==== | ====Relevant Source Code==== | ||
UI: | |||
The sms app | * action_menu.js | ||
* attachment_menu.js | |||
* compose.js - handle UI specifics of message composition | |||
* dialog.js - generic confirm screen | |||
* fixed_header.js | |||
* thread_list_ui.js - track current number of rendered threads | |||
* thread_ui.js - manage threads | |||
* waiting_screen.js | |||
Actions: | |||
* activity_handler.js - main activity handler | |||
* activity_picker.js - to perform actions such as calling/adding/editing a contact, sending a sms/mms/email | |||
* attachments.js - create an attachment object (media source and text) | |||
* contacts.js - find/filter contacts | |||
* blacklist.js - get the blacklisted numbers | |||
* link_action_helper.js - link url/email/phone data in a message | |||
* message_manager.js - send/received messages | |||
* notify.js - notification on incoming message | |||
* recipients.js - manage recipients list | |||
* smil.js - handle SMIL document (layout of MMS message) | |||
* startup.js | |||
Settings: | |||
* settings - default MMS size limitation if no operator specification | |||
Util: | |||
* link_helper.js - replace input strings by anchor links for url, phone, email | |||
* utils.js - time, contact details, font size, carrier tag, etc. | |||
* wbmp.js - decode the Wireless Bitmap image format (.wbmp is Wireless Application Protocol Bitmap Format) | |||
Shared code: | |||
* shared/js/l10n.js | |||
* shared/js/lazy_loader.js | |||
The *_mock.js files are not reviewed since they are test files. | |||
===Permissions==== | |||
The sms app requires the following permissions: | |||
"permissions": { | "permissions": { | ||
"sms":{}, | "sms":{}, | ||
"storage":{}, | |||
"contacts":{ "access": "readonly" }, | "contacts":{ "access": "readonly" }, | ||
"settings":{ "access": "readonly" }, | "settings":{ "access": "readonly" }, | ||
"audio-channel-notification":{}, | "audio-channel-notification":{}, | ||
"desktop-notification":{} | "desktop-notification":{} | ||
} | } | ||
====Web Activity Handlers ==== | ====Web Activity Handlers ==== | ||
The SMS app handles | The SMS app handles two Activities, defined in activity_handler.js: | ||
"activities": { | "activities": { | ||
Line 42: | Line 87: | ||
"filters": { | "filters": { | ||
"type": "websms/sms", | "type": "websms/sms", | ||
"number": { " | "number": { | ||
"pattern":"[\\w\\s+#*().-]{0,50}" | |||
} | |||
}, | }, | ||
"disposition": "window" | "disposition": "window" | ||
}, | |||
"share": { | |||
"filters": { | |||
"type": ["image/*", "audio/*", "video/*"], | |||
"number": 1 | |||
}, | |||
"disposition": "window", | |||
"returnValue": true | |||
} | } | ||
} | } | ||
* new - to deliver the view of a message (phone number, body, contact). | |||
* share - to allow to send a media in attachment via a SMS/MMS, calling the sms app from another app. | |||
====Web Activity Usage ==== | ====Web Activity Usage ==== | ||
* blacklist.js gets the blacklist file by an XHR request to 'js/backlist.json'. | |||
* ActivityPicker can make calls to several activities with web usage: new (mail), view (url) | |||
* A pick Activity (with type ['image/*', 'audio/*', 'video/*']) is used to allow the user to create an attachment. | |||
* A pick Activity (with type webontacts/contacts) is used to retrieve a Contact from the adress book. | |||
==== Notable Event Handlers ==== | ==== Notable Event Handlers ==== | ||
The 'hashchange' event handler is used in threads.js and activity_handler.js. | |||
===Code Review Notes=== | ===Code Review Notes=== | ||
====1. XSS & HTML Injection attacks==== | ====1. XSS & HTML Injection attacks==== | ||
InnerHTML and parseFromString are used with user input (e.g. body of the sms), | |||
but it seems properly escape &<>'" characters (via Utils.Template.prototype.interpolate() and Utils.escapeHTML()). | |||
ISSUE ?: When generating the slides for building the SMIL document (smil.js): the 'blobType' of the document to be attached is used to build | |||
an HTML string by concatenation. | |||
The blobType is just the MIME type returned by Utils.typeFromMimeType(). In the case it detects text, the MIME type is returned as the | |||
string it reads in the field. As it is not escaped to build the HTML string, what if the MIME type has been tampered with ? | |||
Ref: smil.js, l.72 & l.92 | |||
utils.js, l.427 | |||
====2. Secure Communications ==== | ====2. Secure Communications ==== | ||
No comunication with any external services. | |||
====3. Secure data storage ==== | ====3. Secure data storage ==== | ||
Line 73: | Line 139: | ||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
No issues | No issues. | ||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
====6. Interfaces with other Apps/Content==== | ====6. Interfaces with other Apps/Content==== | ||
No issues | No issues. | ||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
* bug 879136 | * bug 879136 | ||
[[Category:SecReview]] | [[Category:SecReview]] |
Revision as of 05:43, 6 September 2013
App Review Details
- App: SMS
- Review Date: 27th August 2013
- Latest Commit: https://github.com/mozilla-b2g/gaia/commit/b4ca01e782c5d03ff4af870625dd2ebcb4ecb17e
- Branch Reviewed : Master
- Review Lead: Stéphanie Ouillon
Overview
SMS app provides the ability to send and receive SMS/MMS messages. SMS was reviewed previously in jun 2013. This review has been completed primarily to include the addition of MMS functionality. The prior review can be found here
Architecture
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.
A message is composed of a phone number and optional body, contact and threadId. There is also web activity handlers so that other apps can ask the SMS app to send a message.
Inputs to the SMS app: SMS events from system, web activity requests, data from contacts, user input to write the SMS.
At startup, 4 components are initialized:
- ActivityHandler: application's global Activity handler to delegate to specific handler based on the Activity name; action when receiving a sms/mms.
- ThreadUI: UI actions (listeners, view initialization, message templating...)
- MessageManager: message actions (send, receive...)
- Settings: set the default mms message size limitation
Relevant Source Code
UI:
- action_menu.js
- attachment_menu.js
- compose.js - handle UI specifics of message composition
- dialog.js - generic confirm screen
- fixed_header.js
- thread_list_ui.js - track current number of rendered threads
- thread_ui.js - manage threads
- waiting_screen.js
Actions:
- activity_handler.js - main activity handler
- activity_picker.js - to perform actions such as calling/adding/editing a contact, sending a sms/mms/email
- attachments.js - create an attachment object (media source and text)
- contacts.js - find/filter contacts
- blacklist.js - get the blacklisted numbers
- link_action_helper.js - link url/email/phone data in a message
- message_manager.js - send/received messages
- notify.js - notification on incoming message
- recipients.js - manage recipients list
- smil.js - handle SMIL document (layout of MMS message)
- startup.js
Settings:
- settings - default MMS size limitation if no operator specification
Util:
- link_helper.js - replace input strings by anchor links for url, phone, email
- utils.js - time, contact details, font size, carrier tag, etc.
- wbmp.js - decode the Wireless Bitmap image format (.wbmp is Wireless Application Protocol Bitmap Format)
Shared code:
- shared/js/l10n.js
- shared/js/lazy_loader.js
The *_mock.js files are not reviewed since they are test files.
Permissions=
The sms app requires the following permissions:
"permissions": { "sms":{}, "storage":{}, "contacts":{ "access": "readonly" }, "settings":{ "access": "readonly" }, "audio-channel-notification":{}, "desktop-notification":{} }
Web Activity Handlers
The SMS app handles two Activities, defined in activity_handler.js:
"activities": { "new": { "filters": { "type": "websms/sms", "number": { "pattern":"[\\w\\s+#*().-]{0,50}" } }, "disposition": "window" }, "share": { "filters": { "type": ["image/*", "audio/*", "video/*"], "number": 1 }, "disposition": "window", "returnValue": true } }
- new - to deliver the view of a message (phone number, body, contact).
- share - to allow to send a media in attachment via a SMS/MMS, calling the sms app from another app.
Web Activity Usage
- blacklist.js gets the blacklist file by an XHR request to 'js/backlist.json'.
- ActivityPicker can make calls to several activities with web usage: new (mail), view (url)
- A pick Activity (with type ['image/*', 'audio/*', 'video/*']) is used to allow the user to create an attachment.
- A pick Activity (with type webontacts/contacts) is used to retrieve a Contact from the adress book.
Notable Event Handlers
The 'hashchange' event handler is used in threads.js and activity_handler.js.
Code Review Notes
1. XSS & HTML Injection attacks
InnerHTML and parseFromString are used with user input (e.g. body of the sms), but it seems properly escape &<>'" characters (via Utils.Template.prototype.interpolate() and Utils.escapeHTML()).
ISSUE ?: When generating the slides for building the SMIL document (smil.js): the 'blobType' of the document to be attached is used to build an HTML string by concatenation. The blobType is just the MIME type returned by Utils.typeFromMimeType(). In the case it detects text, the MIME type is returned as the string it reads in the field. As it is not escaped to build the HTML string, what if the MIME type has been tampered with ? Ref: smil.js, l.72 & l.92
utils.js, l.427
2. Secure Communications
No comunication with any external services.
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.
5. Use of Privileged APIs
6. Interfaces with other Apps/Content
No issues.
Actions & Recommendations
- bug 879136