Security/Reviews/Gaia/Video: Difference between revisions
Jump to navigation
Jump to search
mNo edit summary |
|||
| (14 intermediate revisions by 2 users not shown) | |||
| Line 2: | Line 2: | ||
* App: Video | * App: Video | ||
* Review Date: | * Review Date: 14 Sep 2013 | ||
* | * Latest Commit: https://github.com/mozilla-b2g/gaia/commit/73d7f74142f204241505bd6dfbb84d0ed83c323a | ||
* Branch Reviewed: Master | |||
* Reviewer: Stéphanie Ouillon | |||
Please see page history for details of previous reviews. | |||
=== Overview === | === Overview === | ||
| Line 13: | Line 15: | ||
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 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 | The app exposes a "view" activity that third party apps can use to play videos. This activity accepts links to content of type video/webm, video/mp4, video/3gpp and video/ogg. | ||
Playing videos from Youtube in the Video app is not longer available since https://github.com/mozilla-b2g/gaia/commit/c7133fc4b543ab0223e29ef1bf559d28f80eeaf6 | |||
in response of {{bug|887454}}. Instead, attempting to play a YouTube video will launch it on the YouTube page. | |||
===Architecture=== | ===Architecture=== | ||
| Line 23: | Line 26: | ||
====Relevant Source Code==== | ====Relevant Source Code==== | ||
Source code can be found at https://github.com/mozilla-b2g/gaia/tree/ | Source code can be found at https://github.com/mozilla-b2g/gaia/tree/master/apps/video | ||
Application code: | Application code: | ||
| Line 30: | Line 33: | ||
* view.html - The UI for the view that is shown for the 'view' Activity | * view.html - The UI for the view that is shown for the 'view' Activity | ||
* js/video.js - The code for the main application | * js/video.js - The code for the main application | ||
* js/ | * js/metadata.js - Metadata parsing for video files | ||
* js/ | * js/db.js - Handles the videos list | ||
* js/thumbnail_list.js - The code to render video data into a list | |||
* js/thumbnail_date_group.js - Grouping mechanism | |||
* js/thumbnail_item.js - Render a single video view object | |||
Shared code: | Shared code: | ||
* shared/js/l10n.js | * shared/js/l10n.js | ||
* shared/js/ | * shared/js/l10n_date.js | ||
* shared/js/template.js | |||
* shared/js/device_storage/enumerate_all.js< | |||
* shared/js/mediadb.js | * shared/js/mediadb.js | ||
* shared/js/blobview.js | * shared/js/blobview.js | ||
* shared/js/get_video_rotation | * shared/js/media/get_video_rotation.js | ||
See also these bugs: | See also these bugs: | ||
* bug 840659 [Security Review] Gaia Shared Code MediaDB | |||
* | |||
====Permissions==== | ====Permissions==== | ||
| Line 54: | Line 61: | ||
* "audio-channel-content":{}, | * "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) | * "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 | * "deprecated-hwvideo":{}, | ||
* "audio-channel-content" | * "audio-channel-content":{}, | ||
* systemXHR | * "systemXHR":{} | ||
The access to | The application needs access to both pictures and videos. This is because when you take a video with the Camera app, it will also create a poster image and store that image in the pictures storage. When the Video app deletes a video, that poster image is also deleted, hence the requirement on readwrite to pictures. | ||
The | The access to Settings is questionable and only used for keeping track of the locale. See the bugs filed for that in the Actions & Recommendations section. | ||
The systemXHR permission was required to make a call to the YouTube API which is no longer used. This permission should be removed. | |||
====Web Activity Handlers ==== | ====Web Activity Handlers ==== | ||
| Line 70: | Line 75: | ||
The application makes the following activities available to other apps: | The application makes the following activities available to other apps: | ||
* view - To let third party applications play video files | * view - To let third party applications play video files in a standard UI. | ||
The code handling <code>view</code> is in <code>js/view.js</code>. It only knows how to play movies that you reference by type. The following types are supported: video/webm video/mp4 video/ogg video/3gpp. | |||
* pick - To select a video. Supported types: video/* video/webm video/mp4 video/ogg video/3gpp | |||
* open - To launch a video. Supported types: video/webm video/mp4 video/ogg video/3gpp | |||
====Web Activity Usage ==== | ====Web Activity Usage ==== | ||
The | The app calls the following Web Activities: | ||
* configure - To open the media storage panel when the default storage is unavailable | |||
* record - To launch the Camera app | |||
* share - To send a video, eg. by SMS or Email | |||
==== Notable Event Handlers ==== | ==== Notable Event Handlers ==== | ||
| Line 81: | Line 94: | ||
====1. XSS & HTML Injection attacks==== | ====1. XSS & HTML Injection attacks==== | ||
No XSS or Injection vulnerabilities have been found in the code. | |||
innerHTML is used in thumbnail_item.js and thumbnail_date_group.js with escaped input. | |||
====2. Secure Communications ==== | ====2. Secure Communications ==== | ||
====3. (Secure) data storage ==== | ====3. (Secure) data storage ==== | ||
| Line 99: | Line 107: | ||
* The IndexDB API is used to manage the metadata of the video files (key frame/thumbnail) | * 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 | All the code to do this is contained in /shared/js/mediadb.js. | ||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
| Line 105: | Line 113: | ||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
* DeviceStorage - used to access the | * DeviceStorage - used to access the video and image (posters) files | ||
* Settings - used by shared/js/l10n.js to keep track of locale changes | * Settings - used by shared/js/l10n.js to keep track of locale changes | ||
* SystemXHR | * SystemXHR - no longer used | ||
The usage of Settings API is questionable. See the bugs filed for that in the Actions & Recommendations section. | 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==== | ||
==== 7. Oddities ==== | ==== 7. Oddities ==== | ||
| Line 120: | Line 126: | ||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
* {{bug|932795}} SystemXHR permission should be removed. | |||
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: | 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: | ||
| Line 125: | Line 133: | ||
* {{bug|841071}} Settings are globally shared between applications | * {{bug|841071}} Settings are globally shared between applications | ||
* {{bug|841196}} Applications should stop using settings permission to just get locale info | * {{bug|841196}} Applications should stop using settings permission to just get locale info | ||
Latest revision as of 15:18, 30 October 2013
App Review Details
- App: Video
- Review Date: 14 Sep 2013
- Latest Commit: https://github.com/mozilla-b2g/gaia/commit/73d7f74142f204241505bd6dfbb84d0ed83c323a
- Branch Reviewed: Master
- Reviewer: Stéphanie Ouillon
Please see page history for details of previous reviews.
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 links to content of type video/webm, video/mp4, video/3gpp and video/ogg.
Playing videos from Youtube in the Video app is not longer available since https://github.com/mozilla-b2g/gaia/commit/c7133fc4b543ab0223e29ef1bf559d28f80eeaf6 in response of bug 887454. Instead, attempting to play a YouTube video will launch it on the YouTube page.
Architecture
Components
Relevant Source Code
Source code can be found at https://github.com/mozilla-b2g/gaia/tree/master/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/metadata.js - Metadata parsing for video files
- js/db.js - Handles the videos list
- js/thumbnail_list.js - The code to render video data into a list
- js/thumbnail_date_group.js - Grouping mechanism
- js/thumbnail_item.js - Render a single video view object
Shared code:
- shared/js/l10n.js
- shared/js/l10n_date.js
- shared/js/template.js
- shared/js/device_storage/enumerate_all.js<
- shared/js/mediadb.js
- shared/js/blobview.js
- shared/js/media/get_video_rotation.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":{}
The application needs access to both pictures and videos. This is because when you take a video with the Camera app, it will also create a poster image and store that image in the pictures storage. When the Video app deletes a video, that poster image is also deleted, hence the requirement on readwrite to pictures.
The access to Settings is questionable and only used for keeping track of the locale. See the bugs filed for that in the Actions & Recommendations section.
The systemXHR permission was required to make a call to the YouTube API which is no longer used. This permission should be removed.
Web Activity Handlers
The application makes the following activities available to other apps:
- view - To let third party applications play video files in a standard UI.
The code handling view is in js/view.js. It only knows how to play movies that you reference by type. The following types are supported: video/webm video/mp4 video/ogg video/3gpp.
- pick - To select a video. Supported types: video/* video/webm video/mp4 video/ogg video/3gpp
- open - To launch a video. Supported types: video/webm video/mp4 video/ogg video/3gpp
Web Activity Usage
The app calls the following Web Activities:
- configure - To open the media storage panel when the default storage is unavailable
- record - To launch the Camera app
- share - To send a video, eg. by SMS or Email
Notable Event Handlers
Code Review Notes
1. XSS & HTML Injection attacks
No XSS or Injection vulnerabilities have been found in the code. innerHTML is used in thumbnail_item.js and thumbnail_date_group.js with escaped input.
2. Secure Communications
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.
4. Denial of Service
5. Use of Privileged APIs
- DeviceStorage - used to access the video and image (posters) files
- Settings - used by shared/js/l10n.js to keep track of locale changes
- SystemXHR - no longer used
The usage of Settings API is questionable. See the bugs filed for that in the Actions & Recommendations section.
6. Interfaces with other Apps/Content
7. Oddities
Security Risks & Mitigating Controls
Actions & Recommendations
- bug 932795 SystemXHR permission should be removed.
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