Security/Reviews/Gaia/Gallery: Difference between revisions
Line 133: | 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 | ||
The application might not need the audio-channel-content permission. I've filed a bug to investigate this. | |||
* {{bug|841365}} Gallery might not need the audio-channel-content permission |
Revision as of 15:06, 14 February 2013
App Review Details
- App: Gallery
- Review Date: 11 Feb 2013
- Review Lead: Stefan Arentz
Overview
Gallery is the photo and video browser application. You can see all the photos and videos that you have taken in the past, which are stored on the SD Card. You can browse through them in a thumbnail view and also see full size photos and videos by tapping a thumbnail.
You can also select a single or multiple media items and then share those items with other applications through the 'share' activity. It is also possible to delete items that you have selected.
Last, there is also a simple photo editor in the application where you can do things like adjust the exposure, do a crop and apply filters.
The app does not have photo or video taking abilities. It delegates that to the Camera app.
Architecture
Components
Relevant Source Code
Source code can be found at https://github.com/mozilla-b2g/gaia/tree/v1-train/apps/gallery
Application code:
- index.html - The UI for the application
- open.html - The UI for the view that is shown for the 'open' Activity
- js/open.js - The code for handling the 'open' Activity
- js/gallery.js - The code for the application
- js/ImageEditor.js - Library with image editor functions
- js/MetadataParser.js - Library with functions for thumbnails and metadata collection
Shared code:
- shared/js/gesture_detector.js
- shared/js/mouse_event_shim.js
- shared/js/mediadb.js
- shared/js/blobview.js
- shared/js/media/jpeg_metadata_parser.js
- shared/js/media/get_video_rotation.js
- shared/js/media/video_player.js
- shared/js/media/media_frame.js
- shared/js/visibility_monitor.js
See also these bugs:
- bug 840659 [Security Review] Gaia Shared Code MediaDB
- bug 840660 [Security Review] Gaia Shared Code Media
Permissions
The application has the following permissions:
- "device-storage:pictures":{"access":"readwrite"}, - Because it needs to read and write to the pictures stored on the SD Card
- "device-storage:videos":{"access":"readwrite"}, - Because it needs to read and write to the videos stored on the SD Card
- "audio-channel-content":{}, - This permissions is needed to handle 'multiple audio streams'. I asked :djf but he was also not sure why this was in there.
- "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)
Web Activity Handlers
The application makes the following activities available to other apps:
- browse - To allow a third party app to open the media browser. This has no return value so it is basically just used as a way to open the Gallery app. It is used to open the gallery from the Camera app.
- pick - To allow a third party app to pick photos and videos from the library. Contains the type of media to pick (video or photo). The return value is a list of filename/blob pairs for the selected items.
- open - To open an individual item in the photo (or video?) viewer. It takes a blob as a value.
Blobs are documented at https://developer.mozilla.org/en/docs/DOM/Blob
Web Activity Usage
The following activities are initiated:
- record - Used when the camera button is clicked. It will simply start the camera app. No info passed.
- share - Used when the share button is clicked. It gets passed the blobs, filenames and full paths of selected items.
The *full paths* are really just relative paths to the media files from the root of the specific DeviceStorage location. So for example DCIM/100MZLLA/IMG_00001.JPG for a photo relative to the photos storage. This is no concern.
Notable Event Handlers
Code Review Notes
1. XSS & HTML Injection attacks
No vulnerable or questionable code has been found. The application has no input fields or constructed text labels.
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 video and photo files on the SD Card
- The IndexDB API is used to manage the metadata of the raw 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
Possible DoS attacks:
- Provide an image with corrupt or invalid meta data to crash shared/js/media/jpeg_metadata_parser.js
- Provide an invalid video file to crash shared/js/media/media_frame.js or .../get_video_rotation.js
- Provide extremely large images
- Provide many small images
These attacks need physical access to the device and would just render the app unusable. They would not actually break out of the sandbox or expose private data.
Images can be manipulated directly by accessing the SD Card from an external device. (Through USB or direct)
5. Use of Privileged APIs
- DeviceStorage - used to access the audio and video files
- audio-channel-content - not sure yet why this is in there
- settings - used by shared/js/l10n.js to keep track of locale changes
6. Interfaces with other Apps/Content
Security Risks & Mitigating Controls
Information leaking: allowing third party applications with no explicit GPS permission to see photos with GPS data attached.
Actions & Recommendations
The application leaks GPS location data through EXIF. The following bugs were filed:
- bug 840223 Gallery app should not leak GPS location info
- bug 840271 Gallery exposes GPS EXIF data when sharing photos to third party apps
The application unnecesarily 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 audio-channel-content permission. I've filed a bug to investigate this.
- bug 841365 Gallery might not need the audio-channel-content permission