Security/Reviews/Gaia/Music 2013-02

From MozillaWiki
< Security‎ | Reviews‎ | Gaia
Jump to: navigation, search

App Review Details

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.

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.

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

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

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

The application makes the following activities available to other apps:

  • open - To let third party applications play audio files in a standard UI

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

Web Activity Usage

The following activities are initiated:

  • share

Notable Event Handlers

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

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.

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 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

Only through Web Activities.

7. Oddities

In utils.js there is a escapeHTML() 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 navigator.requestWakeLock('cpu') ... isn't that something that should be behind a permission?

Security Risks & Mitigating Controls

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:

  • 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
Retrieved from "https://wiki.mozilla.org/index.php?title=Security/Reviews/Gaia/Music_2013-02&oldid=714589"