Security/Reviews/Gaia/Music: Difference between revisions

← Older edit

Security/Reviews/Gaia/Music (view source)

Revision as of 19:34, 20 November 2015

458 bytes added , 20 November 2015

→‎App Review Details

Yyang

15

edits

@@ Line 1: / Line 1: @@
 === App Review Details ===
+* App: Music
+* Review Date: 20 Nov 2015
+* Latest Commit: https://github.com/mozilla-b2g/gaia/commit/9563c38e0cd3fbaaec2a40cd525baf7df5098f2f
+* Review Lead: Yu Yang ( https://mozillians.org/en-US/u/yuyang753/ )
-* App: Gallery
+=== Overview ===
-* Review Date: 20 Feb 2013
-* Review Lead: Stefan Arentz
-=== 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.
-Music the music player.
+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/v1-train/apps/music
+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
+*   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/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/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====
 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 ====
-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 ====
 The following activities are initiated:
-* share
+* 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 ====
+====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 ====
 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 90: / Line 119: @@
 ====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
-TODO what is the deprecated-hwvideo permission? Is that an API?
 ====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 it is needed because all text is transferred to content using setText anyway.
-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 ===
@@ Line 108: / Line 129: @@
 === 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
+[[Category:SecReview]]