Issue metadata
Sign in to add a comment
|
Security: Download directory can be set to arbitrary paths via chrome://settings |
||||||||||||||||||||||
Issue descriptionSpin-off from issue 817920 : By directly calling the settingsPrivate.setPref function it is possible to set the user's download directory to above /home/chronos/user/Downloads. If the targeted directory does not exist, it will be created when the first file is downloaded. Only automatic downloads will be placed in this directory, if the user is prompted for a filename, files will be placed in the normal Downloads directory. The following javascript snippet, when executed from the chrome://settings page, will set the Downloads directory to /home/chronos/user/crash chrome.settingsPrivate.setPref('download.default_directory', '/home/chronos/user/crash', '1', console.log) This allows a user to write to arbitrary files in the file system. That's certainly unintentional, but not only low severity given the trust level of the browser process within Chrome OS.
,
Mar 2 2018
is there any value in exposing the download dir pref on CrOS ? i think we should blacklist it entirely.
,
Mar 5 2018
There is value: 1. You can use a subdirectory of their download directory as the default download location 2. You can set drive as the default download location Looking at the code, I think the most robust thing would be to check whether the directory is legit on use (and fall back to the default location if not). That way, the existing code using the pref will continue to work. Adding a few folks who have touched Chrome OS download stuff for their input.
,
Mar 5 2018
Oh, great catch, this is indeed a problem. > Looking at the code, I think the most robust thing would be to check whether the directory is legit on use (and fall back to the default location if not). Sounds good.
,
Mar 5 2018
Here's a CL (with unit tests, otherwise untested): https://chromium-review.googlesource.com/c/chromium/src/+/948852
,
Mar 6 2018
FYI for Download dir integration for ARC++.
,
Mar 9 2018
,
Apr 5 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/441cc658c2f0a4abdd924cdfda00bb78fe196c69 commit 441cc658c2f0a4abdd924cdfda00bb78fe196c69 Author: Mattias Nissler <mnissler@chromium.org> Date: Thu Apr 05 00:08:56 2018 Override download dir in browser_tests Centrally set up a temp directory to be used as download directory. Remove lots of ad-hoc code in individual tests doing the same. While this is a nice cleanup in itself, the main motivation is the plan to restrict download location to only specifically whitelisted directory subtrees on Chrome OS in a subsequent code change without all the existing tests having to worry about that detail. BUG= chromium:818138 TEST=Compiles and passes tests. Change-Id: I88bd945d544c7536c3e1a884928c3e72200efc9d Reviewed-on: https://chromium-review.googlesource.com/953942 Reviewed-by: Min Qin <qinmin@chromium.org> Reviewed-by: Scott Violet <sky@chromium.org> Commit-Queue: Mattias Nissler <mnissler@chromium.org> Cr-Commit-Position: refs/heads/master@{#548263} [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/apps/guest_view/web_view_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/browsing_data/browsing_data_remover_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/download/download_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/download/notification/download_notification_interactive_uitest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/download/save_page_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/extensions/api/extension_action/browser_action_apitest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/extensions/api/streams_private/streams_private_apitest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/extensions/webstore_installer_test.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/extensions/webstore_installer_test.h [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/lifetime/browser_close_manager_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/loader/chrome_resource_dispatcher_host_delegate_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/page_load_metrics/page_load_metrics_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/policy/policy_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/prefs/pref_functional_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/safe_browsing/safe_browsing_navigation_observer_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/sessions/session_restore_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/ssl/ssl_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/browser/ui/search/local_ntp_browsertest.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/base/in_process_browser_test.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/base/in_process_browser_test.h [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/base/ui_test_utils.cc [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/data/chromeos/app_mode/get_volume_list/src/background.js [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/data/chromeos/app_mode/webstore/downloads/aaedpojejpghjkedenggihopfhfijcko.crx [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/data/extensions/api_test/file_browser/mount_test/test.js [modify] https://crrev.com/441cc658c2f0a4abdd924cdfda00bb78fe196c69/chrome/test/data/extensions/api_test/file_system/get_volume_list/background.js
,
Apr 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9191e417b00328d59a7060fa6bbef061a3fe4ce4 commit 9191e417b00328d59a7060fa6bbef061a3fe4ce4 Author: Mattias Nissler <mnissler@chromium.org> Date: Mon Apr 09 11:12:57 2018 Sanitize Chrome OS download directory before use. Make sure the download directory is within a legit location. Switch a couple unit tests that used a random download location to use an override for the default download directory instead. BUG= chromium:818138 TEST=Unit tests Change-Id: I6edd0c82891be3d05b0009dc4eb5a393d0f1cad6 Reviewed-on: https://chromium-review.googlesource.com/948852 Commit-Queue: Mattias Nissler <mnissler@chromium.org> Reviewed-by: Kazuhiro Inaba <kinaba@chromium.org> Reviewed-by: Min Qin <qinmin@chromium.org> Cr-Commit-Position: refs/heads/master@{#549150} [modify] https://crrev.com/9191e417b00328d59a7060fa6bbef061a3fe4ce4/chrome/browser/download/chrome_download_manager_delegate_unittest.cc [modify] https://crrev.com/9191e417b00328d59a7060fa6bbef061a3fe4ce4/chrome/browser/download/download_prefs.cc [modify] https://crrev.com/9191e417b00328d59a7060fa6bbef061a3fe4ce4/chrome/browser/download/download_prefs.h [modify] https://crrev.com/9191e417b00328d59a7060fa6bbef061a3fe4ce4/chrome/browser/download/download_prefs_unittest.cc [modify] https://crrev.com/9191e417b00328d59a7060fa6bbef061a3fe4ce4/chrome/browser/download/download_target_determiner_unittest.cc
,
May 2 2018
,
May 2 2018
,
May 3 2018
Setting component for the sake of posterity.
,
May 30 2018
,
Jun 6 2018
is this related to issue 836362
,
Jun 15 2018
,
Aug 8
This bug has been closed for more than 14 weeks. Removing security view restrictions. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot |
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by sheriffbot@chromium.org
, Mar 2 2018