Confirmed users
971
edits
Security/Reviews/Gaia/Dialer: Difference between revisions
Security/Reviews/Gaia/Dialer (view source)
Revision as of 20:00, 27 February 2013
, 27 February 2013→Overview
| (7 intermediate revisions by the same user not shown) | |||
| Line 1: | Line 1: | ||
=== App Review Details === | === App Review Details === | ||
* App: Dialer, which is part of the Communications application | * App: Dialer, which is part of the Communications application | ||
| Line 7: | Line 5: | ||
* Review Lead: Stefan Arentz | * Review Lead: Stefan Arentz | ||
* Review Bug: {{bug|754741}} [Security Review] B2G Gaia - Dialer | * Review Bug: {{bug|754741}} [Security Review] B2G Gaia - Dialer | ||
* Dependency Tree: https://bugzilla.mozilla.org/showdependencytree.cgi?id=754741&hide_resolved= | * Dependency Tree: https://bugzilla.mozilla.org/showdependencytree.cgi?id=754741&hide_resolved=0 | ||
=== Overview === | === Overview === | ||
This review only looks at the Dialer component of the communications app. | |||
===Architecture=== | ===Architecture=== | ||
| Line 22: | Line 20: | ||
ISSUE: Dialer should be separated from contacts and facebook | ISSUE: Dialer should be separated from contacts and facebook | ||
* {{bug|845945}} Dialer should be turned into minimal standalone application | |||
The dialer has three top level user interfaces: | The dialer has three top level user interfaces: | ||
| Line 231: | Line 231: | ||
====3. (Secure) data storage ==== | ====3. (Secure) data storage ==== | ||
The recent calls database is stored on the device using indexDB. The code for this is abstracted in <code>recents_db.js</code>. Looks pretty solid, no comments. | |||
It also uses asyncStorage, which is implemented in <code>shared/js/async_storage.js</code>. The app stores the Facebook oauth token in there (the contacts app does, but it is checked in the dialer to see if facebook has been 'connected') and the time the call log was last visited. | |||
There is nothing wrong with this approach, except that because the dialer is part of the communications 'umbrella' app, more code than needed has access to these databases. See: | |||
* {{bug|845945}} Dialer should be turned into minimal standalone application | |||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
| Line 260: | Line 268: | ||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
The | The dialer unnecessarily has access to all system settings. This is an issue with the Settings API that should be improved in a future version of Firefox OS: | ||
* {{bug|841071}} Settings are globally shared between applications | * {{bug|841071}} Settings are globally shared between applications | ||
The dialer is embedded in a bigger app, which is not great from a security pov: | |||
* {{bug|845945}} Dialer should be turned into minimal standalone application | |||
Multiple input validation issues that need to be fixed: | |||
* {{bug|845383}} Dialer accepts super long phone number which breaks the phone until reboot | |||
* {{bug|845361}} Dialer does not correctly validate input to the dial activity handler | |||
* {{bug|845045}} Dialer can be tricked into displaying one number but dialing another | |||
* {{bug|845930}} Dialer does not validate phone numbers received via BlueTooth | |||
The dialer does not verify the source of 'postMessage()' messages: | |||
* {{bug|845487}} Dialer responds to cross-origin messages without verifying the source (exploitable) | |||