Confirmed users
152
edits
Security/Reviews/Gaia/sms: Difference between revisions
m
update link to previous review notes
Ptheriault (talk | contribs) |
m (update link to previous review notes) |
||
| (3 intermediate revisions by 2 users not shown) | |||
| Line 1: | Line 1: | ||
=== App Review Details === | === App Review Details === | ||
* App: SMS | * App: SMS | ||
* Review Date: | * Review Date: 3rd March 2014 | ||
* Latest Commit: https://github.com/mozilla-b2g/gaia/commit/ | * Latest Commit: https://github.com/mozilla-b2g/gaia/commit/db3b537b4a29be4849f45fb7ec27b77bb778a2a6 | ||
* Branch Reviewed : Master | * Branch Reviewed : Master | ||
* Reviewers : Stéphanie Ouillon & Paul Theriault | * Reviewers : Stéphanie Ouillon & Paul Theriault | ||
=== Overview === | === Overview === | ||
SMS app provides the ability to send and receive SMS/MMS messages. SMS was reviewed previously in jun 2013. | SMS app provides the ability to send and receive SMS/MMS messages. SMS was reviewed previously in jun 2013. | ||
The main focus of this re-review was to include the addition of subject support in MMS ({{bug|919966}}) and delivery report support ({{bug|919977}}). The prior review can be found [https://wiki.mozilla.org/index.php?title=Security/Reviews/Gaia/sms&oldid=703745 here]. | |||
===Architecture=== | ===Architecture=== | ||
| Line 35: | Line 37: | ||
* dialog.js - generic confirm screen | * dialog.js - generic confirm screen | ||
* fixed_header.js | * fixed_header.js | ||
* information.js - Information view (delivery report) | |||
* thread_list_ui.js - track current number of rendered threads | * thread_list_ui.js - track current number of rendered threads | ||
* thread_ui.js - manage threads | * thread_ui.js - manage threads | ||
| Line 69: | Line 72: | ||
===Permissions=== | ===Permissions=== | ||
The sms app requires the following permissions | The sms app requires the following permissions: | ||
"permissions": { | "permissions": { | ||
"sms":{}, | "sms":{}, | ||
"storage":{}, | "storage":{}, | ||
"mobileconnection": {}, | |||
"contacts":{ "access": "readonly" }, | "contacts":{ "access": "readonly" }, | ||
"settings":{ "access": "readonly" }, | "settings":{ "access": "readonly" }, | ||
"audio-channel-notification":{}, | "audio-channel-notification":{}, | ||
"desktop-notification":{} | "desktop-notification":{}, | ||
"phonenumberservice":{} | |||
} | } | ||
There are two new permissions: | |||
* mobileconnection: used in settings.js for listening to mobile connection datachange before switching between SIMs . | |||
* phonenumberservice: used for normalizing and comparing phone numbers | |||
====Web Activity Handlers ==== | ====Web Activity Handlers ==== | ||
| Line 116: | Line 125: | ||
The 'hashchange' event handler is used in threads.js and activity_handler.js. | The 'hashchange' event handler is used in threads.js and activity_handler.js. | ||
==== Datastore access ==== | |||
Access to Facebook contacts: | |||
"datastores-access": { | |||
"Gaia_Facebook_Friends": { | |||
"description": "Facebook Friends" | |||
} | |||
} | |||
===Code Review Notes=== | ===Code Review Notes=== | ||
====1. XSS & HTML Injection attacks==== | ====1. XSS & HTML Injection attacks==== | ||
Data retrieved for delivery reports seems to be correctly rendered in the information panel. Timestamps come from Gecko. | |||
There is a few "innerHTML" but there are used alongwith templating (TMPL.*.interpolate). | |||
====2. Secure Communications ==== | ====2. Secure Communications ==== | ||
| Line 129: | Line 152: | ||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
No issues. | No issues. | ||
Subject messages are limited to 140 characters. | |||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
| Line 137: | Line 161: | ||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
* bug | * See bug 912885 | ||
[[Category:SecReview]] | [[Category:SecReview]] | ||