Security/Reviews/Gaia/costcontrol: Difference between revisions
Jump to navigation
Jump to search
Line 118: | Line 118: | ||
=== Security Risks & Mitigating Controls === | === Security Risks & Mitigating Controls === | ||
* Extraneous certified permissions in manifest. | * Extraneous certified permissions in manifest. | ||
* 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. | * 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. Since the security implications of innerHTML are so severe, it might be worth auditing instances of innerHTML or reach out to developer to find out more (:kaze) | ||
=== Actions & Recommendations === | === Actions & Recommendations === |
Revision as of 18:53, 30 September 2013
App Review Details
- App: Usage (gaia/apps/costcontrol)
- Review Date: Sept 25, 2013
- Review Lead: Rob Fletcher (:omerta)
Overview
Usage application to see credit and data usage statistics.
Presents the user with a graph of either Mobile or Data usage or both. Also, allows the user to set notifications if usage exceeds a certain amount of usage; the user can also reset the usage amount being tracked.
Architecture
A certified app to be preinstalled on the phone.
Components
Relevant Source Code
Application Code
* debug.html - static file used for debug when DEBUGGING is true * fte.html - first time experience page; where the user lands first time using the app * handle_gaps.html - Handle Gaps * index.html - main app page; contains comments later pulled into HTML tags * message_handler.html - handle system messages * settings.html - view page for usage settings * widget.html - According #gaia, it is displayed in the notification tray of the system app
* js/CostControl.js - CostControl is the singleton in charge of provide data to the views by using asynchronous requests. * js/app.js - The application is in charge of display detailed information about the usage. * js/common.js - Common functions used throughout costcontrol * js/mindgap.js - This module is in charge of keep the historical of SIM changes in order to rebuild the accurate usage of each SIM. * js/view_manager.js - The ViewManager is in charge of simply manage the different views of the applications. * js/widget.js - The widget is in charge of show balance, telephony data and data usage statistics depending on the SIM inserted. * js/views/balance.js - The balance tab is in charge of show balance details and allows the use to manually update or top up via USSD or code. * js/views/datausage.js - The data usage tab is in charge of usage charts of mobile and wi-fi networks. * js/views/telephony.js - The telephony tab is in charge of show telephony and billing cycle information.
- shared/js/async_storage.js
- shared/js/l10n.js
- shared/js/l10n_date.js
- shared/js/lazy_loader.js
- shared/js/notification_helper.js
- shared/js/settings_listener.js
Permissions
- Certified
- "sms":{} - sms-received, sms-sent system message.
- "mobileconnection":{} - access to SIM card, check service status
- "settings":{ "access": "readonly" }
- There are no calls to mozSettings(). This permission appears to be extraneous.
- "networkstats-manage":{} - Obtain statistics of data usage
- "telephony": {}, - telephony-call-ended system message.
- Privileged
- None
- Hosted
- "desktop-notification":{} - Notify user, with desktop notification, they've exceeded usage
- "alarms": {}, - alarm system message
- "storage": {} - use storage without size limitations
Web Activity Handlers
The application makes the following activities available to other apps:
- "costcontrol/balance" - simply change hash to #balance-tab
- "costcontrol/telephony" - simply change hash to #telephony-tab
- "costcontrol/data_usage"- simply change hash to #datausage-tab
Web Activity Usage
The following activities are initiated:
- dial - Used to dial webtelephony/number when using "Top Up and Pay". When user wants to add credit, taps "Top Up and Pay", the dial activity is initiated with the number associated with adding credit.
- costcontrol/balance
- costcontrol/telephony
- costcontrol/data_usage
Notable Event Handlers
Code Review Notes
1. XSS & HTML Injection attacks
- No vulnerable code has been found. The application only builds HTML from static sources already defined. No user input is used to generate HTML elements or attributes.
Suspicious but OK
~/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
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; - 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'); - 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
<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
them and shoving them into innerHTML doesn't work. So they have to redefine the script tags and append them.
2. Secure Communications
- This app does not communicate with any external services.
3. Secure data storage
- This application uses localStorage very minimally to store strings that are used later, presumably, in URLs. The app only uses localStorage["sync"].
4. Denial of Service
- There appear to be no avenues for DoS attacks.
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" }
- There are no calls to mozSettings(). This permission appears to be extraneous.
- "networkstats-manage":{}
- Obtain statistics of data usage
- "telephony": {}
- telephony-call-ended system message.
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
- Extraneous certified permissions in manifest.
- After speaking with developer regarding 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. Since the security implications of innerHTML are so severe, it might be worth auditing instances of innerHTML or reach out to developer to find out more (:kaze)