Security/Reviews/Gaia/Music: Difference between revisions

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to navigation Jump to search
No edit summary
 
(19 intermediate revisions by 2 users not shown)
Line 1: Line 1:
=== App Review Details ===
=== App Review Details ===
* App: Music
* App: Music
* Review Date: 20 Feb 2013
* Review Date: 20 Nov 2015
* Review Lead: Stefan Arentz
* Latest Commit: https://github.com/mozilla-b2g/gaia/commit/9563c38e0cd3fbaaec2a40cd525baf7df5098f2f
* Review Bug: {{bug|754745}} [Security Review] B2G Gaia - Music
* Review Lead: Yu Yang ( https://mozillians.org/en-US/u/yuyang753/ )
* 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.
Music is the music player application. It plays music stored on the SD card and also allows to share music via NFC and Bluetooth.


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.
The app keeps a database containing references to audio files and meta data. It uses musicdb to access the files and has code to parse FLAC/ID3/Ogg/MP4 meta data like song title, artist, album, etc. It can also extract the album art from a file.


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.
===Architecture===


The app is aware of headphones being plugged in/out using the mozAudioChannelManager API.


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.
====Components====
 
These components are used and details can be found in https://gaia-components.github.io/
===Architecture===


====Components====
  bridge/
  dom-scheduler/
  fast-list/
  font-fit/
  gaia-component/
  gaia-dialog/
  gaia-fast-list/
  gaia-header/
  gaia-icons/
  gaia-sub-header/
  gaia-text-input/
  gaia-theme/
  gaia-toolbar/
  poplar/
  serviceworkerware/
  sww-raw-cache/


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


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


Application code:
Application code:
 
 
* index.html - The UI for the application
*   index.html - The UI for the application
* open.html - The UI for the view that is shown for the 'open' Activity
*   open.html - The UI for the view that is shown for the 'open' Activity
* js/music.js - The code for the main application
*   pick.html -  The UI for the view that is shown for the 'pick' Activity
* js/open.js - The code that handles the 'open' intent
js/app.js - The code for the main application
* js/Player.js - Code for a music player component
*   js/db.js - Code for musicdb API to manage audio files and metadata
* js/utils.js - Code for misc utilities like string formatting and html escaping
*   js/endpoint.js - Code for music service API
* js/metadata.js - Code to parse ID3/Ogg/MP4 meta data
*  js/nfc_share.js - Code for sharing songs via NFC
*   js/queue.js - Code for Playback Queue
*  js/remote.js – Code for updating remote playback status and metadata (synchronize)
*   js/view.js - Code to share with all diferent views
*  js/shims/device-storage.js - Code for device storage
*  js/metadata/ - Code for metadata parser that supports different formats of metadata
*  js/services/  -  Code for different services like database service, playlist service, etc.
*  components/
*  elements/
*  views/  - Code for different views like albums, artists, playlists, songs,..etc
*  sw.js - Service worker code


Shared code:
Shared code:


* shared/js/l10n.js
* shared/js/media/remote_controls.js
* shared/js/mouse_event_shim.js
* shared/js/async_storage.js
* shared/js/bluetooth_helper.js
* shared/js/image_utils.js
* shared/js/intl_helper.js
* shared/js/intl/l20n-client.js
* shared/js/intl/l20n-service.js
* shared/js/lazy_loader.js
* shared/js/mediadb.js
* shared/js/mediadb.js
* shared/js/blobview.js
* shared/js/moz_intl.js
* shared/js/async_storage.js
* shared/js/omadrm/fl.js
 
* shared/js/text_normalizer.js
See also these bugs:
 
* {{bug|840659}} [Security Review] Gaia Shared Code MediaDB


====Permissions====
====Permissions====


The application has the following permissions:
The application has the following permissions:  


* "device-storage:music":{"access":"readwrite"},
"audio-channel-content": {},
* "audio-channel-content":{},
"bluetooth": {},
* "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)
"device-storage:music":   { "access": "readwrite" },
* deprecated-hwvideo
"device-storage:pictures": { "access": "readwrite" },
"nfc-share": {},
"settings": { "access": "readonly" },
"themeable": {},
"moz-extremely-unstable-and-will-change-webcomponents": {}


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


The application makes the following activities available to other apps:
Support two activities: open (audio) and pick (audio)
 
* open - To let third party applications play audio files in a standard UI
* Open – Open an audio file
 
* Pick – Pick a song and return its playback status like title, artist, album.
Blobs are documented at https://developer.mozilla.org/en/docs/DOM/Blob


====Web Activity Usage ====
====Web Activity Usage ====


The following activities are initiated:
The following activities are initiated:
 
     
* share
* share (endpoint.js)


==== Notable Event Handlers ====
==== Notable Event Handlers ====
No issues identified


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


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


====2. Secure Communications ====
====2. Secure Communications ====
This app does not communicate with any external services.
This app does not communicate with any external services.


====3. (Secure) data storage ====
====3. Secure data storage ====
 
The musicdb API is implemented in /js/db.js and used to manage music files and metadata on the SD Card
Two data storage APIs are used:
 
* The DeviceStorage API is used to manage the raw music files on the SD Card
* 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.
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.


Line 100: Line 119:


====5. Use of Privileged APIs ====
====5. Use of Privileged APIs ====
 
* DeviceStorage - used to access the audio and picture files
* DeviceStorage - used to access the audio and video files
*     Settings - used to read locale settings
* 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.
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 129:
=== 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-02]]
* {{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]]

Latest revision as of 19:34, 20 November 2015

App Review Details

Overview

Music is the music player application. It plays music stored on the SD card and also allows to share music via NFC and Bluetooth.

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

Architecture

Components

These components are used and details can be found in https://gaia-components.github.io/

 bridge/
 dom-scheduler/
 fast-list/
 font-fit/
 gaia-component/
 gaia-dialog/
 gaia-fast-list/
 gaia-header/
 gaia-icons/
 gaia-sub-header/
 gaia-text-input/
 gaia-theme/
 gaia-toolbar/
 poplar/
 serviceworkerware/
 sww-raw-cache/

Relevant Source Code

Source code can be found at https://github.com/mozilla-b2g/gaia/tree/master/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
  • pick.html - The UI for the view that is shown for the 'pick' Activity
  • js/app.js - The code for the main application
  • js/db.js - Code for musicdb API to manage audio files and metadata
  • js/endpoint.js - Code for music service API
  • js/nfc_share.js - Code for sharing songs via NFC
  • js/queue.js - Code for Playback Queue
  • js/remote.js – Code for updating remote playback status and metadata (synchronize)
  • js/view.js - Code to share with all diferent views
  • js/shims/device-storage.js - Code for device storage
  • js/metadata/ - Code for metadata parser that supports different formats of metadata
  • js/services/ - Code for different services like database service, playlist service, etc.
  • components/
  • elements/
  • views/ - Code for different views like albums, artists, playlists, songs,..etc
  • sw.js - Service worker code

Shared code:

  • shared/js/media/remote_controls.js
  • shared/js/async_storage.js
  • shared/js/bluetooth_helper.js
  • shared/js/image_utils.js
  • shared/js/intl_helper.js
  • shared/js/intl/l20n-client.js
  • shared/js/intl/l20n-service.js
  • shared/js/lazy_loader.js
  • shared/js/mediadb.js
  • shared/js/moz_intl.js
  • shared/js/omadrm/fl.js
  • shared/js/text_normalizer.js

Permissions

The application has the following permissions:

"audio-channel-content": {},
"bluetooth": {},
"device-storage:music":    { "access": "readwrite" },
"device-storage:pictures": { "access": "readwrite" },
"nfc-share": {},
"settings": { "access": "readonly" },
"themeable": {},
"moz-extremely-unstable-and-will-change-webcomponents": {}

Web Activity Handlers

Support two activities: open (audio) and pick (audio)

  • Open – Open an audio file
  • Pick – Pick a song and return its playback status like title, artist, album.

Web Activity Usage

The following activities are initiated:

  • share (endpoint.js)

Notable Event Handlers

No issues identified

Code Review Notes

1. XSS & HTML Injection attacks

No XSS or Injection attacks were found.

2. Secure Communications

This app does not communicate with any external services.

3. Secure data storage

The musicdb API is implemented in /js/db.js and used to manage music files and metadata on the SD Card

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

  • DeviceStorage - used to access the audio and picture files
  • Settings - used to read locale settings

6. Interfaces with other Apps/Content

Only through Web Activities.

Security Risks & Mitigating Controls

Actions & Recommendations

Previous Review

Security/Reviews/Gaia/Music_2013-02