Security/Reviews/Gaia/Video

< Security‎ | Reviews‎ | Gaia
Revision as of 21:02, 20 February 2013 by St3fan (talk | contribs) (Created page with "=== App Review Details === * App: Video * Review Date: 20 Feb 2013 * Review Lead: Stefan Arentz === Overview === Video is the Video Player application. It playes videos that a...")
(diff) ← Older revision | Latest revision (diff) | Newer revision → (diff)

App Review Details

  • App: Video
  • Review Date: 20 Feb 2013
  • Review Lead: Stefan Arentz

Overview

Video is the Video Player application. It playes videos that are stored on the SD card. It does not record videos.

The app presents a list view of available videos. Long tapping on a video opens a dialog asking if you want to delete the video. Short apping the video opens a player that immediately starts playing the video. The player also has a delete button and player controls (pause and a scrubber).

The app exposes a "view" activity that third party apps can use to play videos. This activity accepts blobs of type video/webm, video/mp4 and video/3gpp.

The "view" activity also accepts a type of video/youtube. In that case the data is a link to a YouTube page. The app will call a YouTube API using XHR to request links to the available streams for the video and then pick and play the most appropriate one.

Architecture

Components

Relevant Source Code

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

Application code:

  • index.html - The UI for the application
  • view.html - The UI for the view that is shown for the 'view' Activity
  • js/video.js - The code for the main application
  • js/view.js - The code that handles the 'view' intent
  • js/youtube.js - Utility code to deal with the YouTube API

Shared code:

  • shared/js/l10n.js
  • shared/js/mouse_event_shim.js
  • shared/js/mediadb.js
  • shared/js/blobview.js
  • shared/js/get_video_rotation.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:pictures":{"access":"readwrite"},
  • "device-storage:videos":{"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
  • "audio-channel-content"
  • systemXHR

Web Activity Handlers

The application makes the following activities available to other apps:

  • view - To let third party applications play video files and YouTube links in a standard UI

Web Activity Usage

The application does not start any Web Activities.

Notable Event Handlers

Code Review Notes

1. XSS & HTML Injection attacks

2. Secure Communications

The application talks to the YouTube API at: http://www.youtube.com/get_video_info

ISSUE: This API is also available on SSL and that should be used instead.

ISSUE: The code that parses the video id out of the original URL passed to the activity is not very robust and can lead to parameter injection to the get_video_info API.

ISSUE: The get_video_info API might not be an official API for which we have permission to use it.

3. (Secure) data storage

Two data storage APIs are used:

  • The DeviceStorage API is used to manage the video files on the SD Card
  • The IndexDB API is used to manage the metadata of the video files (key frame/thumbnail)

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

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

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

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 device-storage:pictures permission. I've filed a bug to investigate this.

  • bug 843144 Videos might not need the device-storage:pictures permission