Confirmed users
353
edits
(32 intermediate revisions by 2 users not shown) | |||
Line 1: | Line 1: | ||
=== App Review Details === | === App Review Details === | ||
* App: Usage (gaia/apps/costcontrol) | * App: Usage (gaia/apps/costcontrol) | ||
* Review Date: Sept 2013 | * Review Date: Sept 25, 2013 | ||
* Review Lead: Rob Fletcher (:omerta) | * Review Lead: Rob Fletcher (:omerta) | ||
Line 44: | Line 44: | ||
====Permissions==== | ====Permissions==== | ||
*Certified | |||
* "sms":{} - sms-received, sms-sent system message. | ** "sms":{} - sms-received, sms-sent system message. | ||
* "mobileconnection":{} - access to SIM card, check service status | ** "mobileconnection":{} - access to SIM card, check service status | ||
* "settings":{ "access": "readonly" } | ** "settings":{ "access": "readonly" } | ||
** There are no calls to mozSettings(). This permission appears to be extraneous. | *** There are no calls to mozSettings(). This permission appears to be extraneous. | ||
* "networkstats-manage":{} - Obtain statistics of data usage | **** mozSettings is used in shared code:l10n.js, settings_listener.js | ||
* "telephony": {}, - telephony-call-ended system message. | ** "networkstats-manage":{} - Obtain statistics of data usage | ||
** "telephony": {}, - telephony-call-ended system message. | |||
*Privileged | |||
None | **None | ||
*Hosted | |||
* "desktop-notification":{} - Notify user, with desktop notification, they've exceeded usage | ** "desktop-notification":{} - Notify user, with desktop notification, they've exceeded usage | ||
* "alarms": {}, - alarm system message | ** "alarms": {}, - alarm system message | ||
* "storage": {} - use storage without size limitations | ** "storage": {} - use storage without size limitations | ||
====Web Activity Handlers ==== | ====Web Activity Handlers ==== | ||
Line 80: | Line 80: | ||
=====Suspicious but OK===== | =====Suspicious but OK===== | ||
~/work/code/gaia/apps/costcontrol/js/view_manager.js:111 - panel.innerHTML = panel.childNodes[i].nodeValue; | [https://github.com/mozilla-b2g/gaia/blob/master/apps/costcontrol/js/view_manager.js#L111 ~/work/code/gaia/apps/costcontrol/js/view_manager.js:111] - panel.innerHTML = panel.childNodes[i].nodeValue; | ||
- | - To increase performance of the first draw, developers define static HTML inside<br> comment nodes and then later grab that HTML and input to innerHTML. | ||
~/work/code/gaia/apps/costcontrol/js/settings/settings.js:131 - src.innerHTML = xhr.responseText; | [https://github.com/mozilla-b2g/gaia/blob/master/apps/costcontrol/js/settings/settings.js#L131 ~/work/code/gaia/apps/costcontrol/js/settings/settings.js:131] - src.innerHTML = xhr.responseText; | ||
- XHR is fetching /debug.html which has no variable data | - XHR is fetching /debug.html which has no variable data | ||
~/work/code/B2G/gaia/apps/costcontrol/js/view_manager.js:138 - var script = document.createElement('script'); | [https://github.com/mozilla-b2g/gaia/blob/master/apps/costcontrol/js/view_manager.js#L138 ~/work/code/B2G/gaia/apps/costcontrol/js/view_manager.js:138] - var script = document.createElement('script'); | ||
- finds all defined script tags and redfines them, then appends to page | - finds all defined script tags and redfines them, then appends to page | ||
- <script type="text/javascript" defer="" src="js/fte.js"></script> is redefined as <br> <script src="js/fte.js" id="js/fte.js" type="application/javascript"></script> | - <script type="text/javascript" defer="" src="js/fte.js"></script> is redefined as <br> <script src="js/fte.js" id="js/fte.js" type="application/javascript"></script> | ||
- After speaking with a developer, they must redefine script tags because simply uncommenting <br> them and shoving them into innerHTML doesn't work. So they have to redefine the script tags and append them. | |||
=====Notes===== | |||
* After speaking with developer regarding [https://wiki.mozilla.org/Security/Reviews/Gaia/costcontrol&section=20#Suspicious_but_OK suspected but ok] issues, specifically the dynamically creating <script> tags in view_manager.js, I've learned that in some instances developers depend on innerHTML quirks for "sanitization" purposes. | |||
====2. Secure Communications ==== | ====2. Secure Communications ==== | ||
Line 97: | Line 101: | ||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
* There appear to be no avenues for DoS attacks. | |||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
* "sms":{} | sms-sent, sms-received | |||
** Send SMS to request balance. | |||
** TopUp SMS to request credit. | |||
** Check format of SMS to check for balance or topup sms request. | |||
* "mobileconnection":{} | |||
** Access to SIM card, check service status | |||
* "settings":{ "access": "readonly" } | |||
** mozSettings is used in shared code:l10n.js, settings_listener.js | |||
* "networkstats-manage":{} | |||
** Obtain statistics of data usage | |||
* "telephony": {} | |||
** telephony-call-ended system message. | |||
====6. Interfaces with other Apps/Content==== | ====6. Interfaces with other Apps/Content==== | ||
* This app is self contained and does not attempt to receive/send content from other apps. | |||
=== Security Risks & Mitigating Controls === | === Security Risks & Mitigating Controls === |