Confirmed users
353
edits
Security/Reviews/Gaia/costcontrol: Difference between revisions
Security/Reviews/Gaia/costcontrol (view source)
Revision as of 17:32, 9 October 2013
, 9 October 2013→1. XSS & HTML Injection attacks
| (24 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 49: | Line 49: | ||
** "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. | ||
**** mozSettings is used in shared code:l10n.js, settings_listener.js | |||
** "networkstats-manage":{} - Obtain statistics of data usage | ** "networkstats-manage":{} - Obtain statistics of data usage | ||
** "telephony": {}, - telephony-call-ended system message. | ** "telephony": {}, - telephony-call-ended system message. | ||
| Line 79: | 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 99: | Line 104: | ||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
* "sms":{} | * "sms":{} | sms-sent, sms-received | ||
** Send SMS to request balance. | ** Send SMS to request balance. | ||
* "mobileconnection":{} | ** 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" } | * "settings":{ "access": "readonly" } | ||
** | ** mozSettings is used in shared code:l10n.js, settings_listener.js | ||
* "networkstats-manage":{} | * "networkstats-manage":{} | ||
* "telephony": {} | ** 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 === | ||