Security/Reviews/Gaia/Music: Difference between revisions

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to navigation Jump to search
No edit summary
No edit summary
Line 1: Line 1:
=== App Review Details ===
=== App Review Details ===
 
* App:  
* App: Music
* Review Date:  
* Review Date: 20 Feb 2013
* Review Lead:  
* Review Lead: Stefan Arentz
* Review Bug: {{bug|754745}} [Security Review] B2G Gaia - Music
* Resulting Bugs: https://bugzilla.mozilla.org/showdependencytree.cgi?id=754745&hide_resolved=1


=== Overview ===
=== Overview ===


Music is the music player application. It plays music stored on the SD card.


The app keeps a database containing references to audio files and meta data in sync. It uses DeviceStorage to access the files and has code to parse ID3/Ogg/MP4 meta data like song title, artist, album, etc. It can also extract the album art from a file.
===Architecture===


The app can also be used by third-party applications to play files. Using an intent other apps can send a blob to the Music app which will then open a player.


The app is aware of headphones being plugged in/out using the mozAudioChannelManager API.
====Components====
 
Doing a long press on a song invokes starts a 'share' intent. Since the only application that can receive shares of audio content is the system Bluetooth app, the user will be presented with a bluetooth file transfer screen. Unfortunately I was unable to test this because although my Mac and Unagi phones were correctly paired, doing a transfer of a song failed.
 
===Architecture===


====Components====


====Relevant Source Code====
====Relevant Source Code====


Source code can be found at https://github.com/mozilla-b2g/gaia/tree/v1-train/apps/music


Application code:
* index.html - The UI for the application
* open.html - The UI for the view that is shown for the 'open' Activity
* js/music.js - The code for the main application
* js/open.js - The code that handles the 'open' intent
* js/Player.js - Code for a music player component
* js/utils.js - Code for misc utilities like string formatting and html escaping
* js/metadata.js - Code to parse ID3/Ogg/MP4 meta data
Shared code:
* shared/js/l10n.js
* shared/js/mouse_event_shim.js
* shared/js/mediadb.js
* shared/js/blobview.js
* shared/js/async_storage.js
See also these bugs:
* {{bug|840659}} [Security Review] Gaia Shared Code MediaDB


====Permissions====
====Permissions====
The application has the following permissions:
* "device-storage:music":{"access":"readwrite"},
* "audio-channel-content":{},
* "settings":{"access":"readonly"} - Because it needs to access the chosen locale, which is stored in the settings. (This is via shared/js/l10n.js, which accesses the language.current setting through navigator.mozSettings)
* deprecated-hwvideo


====Web Activity Handlers ====
====Web Activity Handlers ====


The application makes the following activities available to other apps:


* open - To let third party applications play audio files in a standard UI
====Web Activity Usage ====


Blobs are documented at https://developer.mozilla.org/en/docs/DOM/Blob


====Web Activity Usage ====
==== Notable Event Handlers ====
 
The following activities are initiated:


* share


==== Notable Event Handlers ====


===Code Review Notes===
===Code Review Notes===


====1. XSS & HTML Injection attacks====
====1. XSS & HTML Injection attacks====
No XSS or Injection attacks were found.


====2. Secure Communications ====
====2. Secure Communications ====


This app does not communicate with any external services.
====3. (Secure) data storage ====
Two data storage APIs are used:


* The DeviceStorage API is used to manage the raw music files on the SD Card
====3. Secure data storage ====
* The IndexDB API is used to manage the metadata of the files (titles, filenames, thumbnails)


All the code to do this is contained in /shared/js/mediadb.js which will have its own review through {{bug|840659}}.


====4. Denial of Service ====
====4. Denial of Service ====


It might be possible to confuse the meta-data parser by storing a malformed or constructed audio file on the device. This could lead to the library failing to render or audio files missing. None of which is serious enough to consider.
Since all file parsing is done in high level JavaScript, there is no way that the above attack could lead to a privilege escalation or code execution attack.


====5. Use of Privileged APIs ====
====5. Use of Privileged APIs ====


* DeviceStorage - used to access the audio and video files
* Settings - used by shared/js/l10n.js to keep track of locale changes
The usage of Settings API is questionable. See the bugs filed for that in the  Actions & Recommendations section.


====6. Interfaces with other Apps/Content====
====6. Interfaces with other Apps/Content====


Only through Web Activities.
==== 7. Oddities ====
In <code>utils.js</code> there is a <code>escapeHTML()</code> function but it is never used. Not sure if that is a problem because all text is transferred to content using setText anyway. Might be a leftover from an older version.


It is possible for an app to do a <code>navigator.requestWakeLock('cpu')</code> ... isn't that something that should be behind a permission?


=== Security Risks & Mitigating Controls ===
=== Security Risks & Mitigating Controls ===
Line 120: Line 53:
=== Actions & Recommendations ===
=== Actions & Recommendations ===


The application 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:
=== Previous Review ===
 
/Security/Reviews/Gaia/Music_2013_February
* {{bug|841071}}  Settings are globally shared between applications
* {{bug|841196}}  Applications should stop using settings permission to just get locale info
 
The application might not need the deprecated-hwvideo permission. I've filed a bug to investigate this.
 
* {{bug|843144}}  Music might not need the deprecated-hwvideo permission
 
The Bluetooth file transfer does not work:


* {{bug|843297}}  Sharing a music file over bluetooth fails
[[Category:SecReview]]

Revision as of 09:17, 19 September 2013

App Review Details

  • App:
  • Review Date:
  • Review Lead:

Overview

Architecture

Components

Relevant Source Code

Permissions

Web Activity Handlers

Web Activity Usage

Notable Event Handlers

Code Review Notes

1. XSS & HTML Injection attacks

2. Secure Communications

3. Secure data storage

4. Denial of Service

5. Use of Privileged APIs

6. Interfaces with other Apps/Content

Security Risks & Mitigating Controls

Actions & Recommendations

Previous Review

/Security/Reviews/Gaia/Music_2013_February

Retrieved from "https://wiki.allizom.org/index.php?title=Security/Reviews/Gaia/Music&oldid=711477"