Confirmed users
152
edits
Security/Reviews/Gaia/Gallery: Difference between revisions
Review for 1.2
(Review for 1.2) |
|||
| Line 2: | Line 2: | ||
* App: Gallery | * App: Gallery | ||
* Review Date: | * Review Date: 14th Oct 2013 | ||
* Review Lead: | * Latest Commit: https://github.com/mozilla-b2g/gaia/commit/4779207aed4cfc3bf7eb9e7221b2901d29b2e0cd | ||
* Branch reviewed: master | |||
* Review Lead: Stephanie Ouillon | |||
Please see page history for details of previous reviews | |||
=== Overview === | === Overview === | ||
| Line 13: | Line 17: | ||
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. | 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. | The app does not have photo or video taking abilities. It delegates that to the Camera app (throught the 'record' Activity). | ||
===Architecture=== | ===Architecture=== | ||
| Line 20: | Line 24: | ||
====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/gallery | ||
* 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 | ||
| Line 30: | Line 31: | ||
* js/ImageEditor.js - Library with image editor functions | * js/ImageEditor.js - Library with image editor functions | ||
* js/MetadataParser.js - Library with functions for thumbnails and metadata collection | * js/MetadataParser.js - Library with functions for thumbnails and metadata collection | ||
* js/frames.js - The code related to fullscreen view | |||
* js/imagesize.js - The code to determine the pixel dimension | |||
Shared code: | Shared code: | ||
* shared/js/l10n.js | |||
* shared/js/device_storage/enumerate_all.js | |||
* shared/js/lazy_loader.js | |||
* shared/js/media/media_utils.js | |||
* shared/js/mediadb.js | |||
* shared/js/visibility_monitor.js | |||
* shared/js/gesture_detector.js | * shared/js/gesture_detector.js | ||
* shared/js/blobview.js | * shared/js/blobview.js | ||
* shared/js/media/media_frame.js | |||
* shared/js/mime_mapper.js | |||
* shared/js/media/jpeg_metadata_parser.js | * shared/js/media/jpeg_metadata_parser.js | ||
* shared/js/ | * shared/js/device_storage/get_storage_if_available.js | ||
* shared/js/ | * shared/js/device_storage/get_unused_filename.js | ||
See also these bugs: | See also these bugs: | ||
| Line 52: | Line 59: | ||
The application has the following permissions: | The application has the following permissions: | ||
* "storage":{} - storage permission to mediadb and async storage client | |||
* "device-storage:pictures":{"access":"readwrite"}, - Because it needs to read and write to the pictures stored on the SD Card | * "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 | * "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 | * "audio-channel-content":{}, - This permissions is needed to interrupt background music if a video is read (see {{bug|841365}}) | ||
* "deprecated-hwvideo":{}, - Needed for sharing video decoding hardware among Camera, Gallery and Video. | |||
* "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) | ||
| Line 93: | Line 102: | ||
* The DeviceStorage API is used to manage the raw video and photo files on the SD Card | * The DeviceStorage API is used to manage the raw video and photo files on the SD Card | ||
* The | * The IndexedDB 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 | All the code to do this is contained in /shared/js/mediadb.js. | ||
====4. Denial of Service ==== | ====4. Denial of Service ==== | ||
imagesize.js checks against corrupt image metadata and reject files that are too large. | |||
====5. Use of Privileged APIs ==== | ====5. Use of Privileged APIs ==== | ||
| Line 118: | Line 118: | ||
=== Security Risks & Mitigating Controls === | === Security Risks & Mitigating Controls === | ||
=== Actions & Recommendations === | === Actions & Recommendations === | ||
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: | 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: | ||
| Line 132: | Line 125: | ||
* {{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 | ||