Security/Reviews/Gaia/costcontrol: Difference between revisions

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to navigation Jump to search
 
(5 intermediate revisions by 2 users not shown)
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 89: Line 90:
         - <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.
         - 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&amp;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 107: Line 111:
** Access to SIM card, check service status
** 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.  
** mozSettings is used in shared code:l10n.js, settings_listener.js
* "networkstats-manage":{}
* "networkstats-manage":{}
** Obtain statistics of data usage
** Obtain statistics of data usage
Line 117: Line 121:


=== Security Risks & Mitigating Controls ===
=== Security Risks & Mitigating Controls ===
* Extraneous certified permissions in manifest.
* After speaking with developer regarding [https://wiki.mozilla.org/Security/Reviews/Gaia/costcontrol&amp;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 ===

Latest revision as of 17:32, 9 October 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 Code
  • 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.
        • mozSettings is used in shared code:l10n.js, settings_listener.js
    • "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.
Notes
  • 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.

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" }
    • 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

  • This app is self contained and does not attempt to receive/send content from other apps.

Security Risks & Mitigating Controls

Actions & Recommendations