New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 840341 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug



Sign in to add a comment

User not warned when setting too-broad default download directory

Reported by ma7h1a...@gmail.com, May 7 2018

Issue description

suppose user set the default download folder to "D:\"
and an auto-start program is in "D:\myexe\main.exe"

when we attempt to download a file named "myexe/main.exe"

<?php
header("Content-Disposition:attachment;filename=myexe/main.exe");
?>

we would surely blocked and get a file named "myexe_main.exe" (x1.jpg)

but if we use chrome.downloads.download instead, then we could overwrite it

chrome.browserAction.onClicked.addListener(tab => {
  var downloadId;

  chrome.downloads.download({
    url: "data:,pwned",
    filename: "myexe/main.exe",
    conflictAction: "overwrite"
  }, id => downloadId = id);
});

then we download a file named "main.exe" (x2.jpg)

let's see the "D:\myexe\main.exe" , find that it was overwritten (x3.jpg)

then we bypass the sandbox without any security notification.

and I think maybe the attack condition is similar to  issue 793620 
 
x3.jpg
22.6 KB View Download
Components: UI>Browser>Downloads Platform>Extensions>API
I believe it's working as expected that the Download API allows saving to any descendent path of the configured downloads folder, although there has been some discussion of how valuable that capability is for legitimate use. 

This isn't really a Sandbox Escape in any meaningful sense, insofar as a user could've configured a file on the Downloads folder itself to autorun. 
then, i would like to know why it's necessary for the chrome downloader transform / to _ , since user could confirm autorun.
re #1
sorry for my rudness , but still , i'd like to talk about my opinion

1. the common points of this issue and  issue 793620  is that there is not any nofication about the operation. 

2.user is not an expert. they may not know about autorun program by default setting , what's more , overwrite exe file that user would use is dangerous.

3. overwrite is a sensetive operation , since not just autorun program but also other config file maybe changed. so that maybe user need to confirm the operation by themself.

4. (x2.jpg) shows that the UI do not notify user about the real path that file was downloaded. so there is also a confusion problem

another consideration: 
if user set the download directory to a root path like "D:\" 
then there is so many files to be overwritten, and easy to caused a code execution.

I think it's not a bad idea to add a notification in the Download API, it could not only rerserve the overwrite feature but also prevent user from attack.
Labels: M-68 Security_Severity-Low Security_Impact-Stable Pri-2
Owner: est...@chromium.org
Bouncing to "enamel" team that handles security UI design questions.  Please evaluate whether there is anything we can do here to protect the user, or close as appropriate if not.
Setting serverity "low" due to the substantial supposition about setting directory to "D:"
Labels: OS-Windows
Project Member

Comment 8 by sheriffbot@chromium.org, May 8 2018

Status: Assigned (was: Unconfirmed)
Project Member

Comment 9 by sheriffbot@chromium.org, Sep 5

Labels: -M-68 M-69 Target-69
Cc: cthomp@chromium.org mea...@chromium.org est...@chromium.org
Labels: Team-Security-UX
Owner: ----
Status: Available (was: Assigned)
Any input from Enamel?
Cc: dtrainor@chromium.org vakh@chromium.org
Components: UI>Settings
The behavior of the downloads extension API seems to be working as intended. I'm not sure this issue is big enough to justify breaking existing functionality of the |conflictAction: "overwrite"| mode of the API.

Additionally, if an extension downloads to an executable file, this should be checked by Safe Browsing, as it would for a normal browser download. +vakh@ to confirm.

I think the remaining concern is that some users might accidentally set their default download directory to be too permissive. It might be worth thinking about if we should add a warning/note to Settings to alert a user who chooses e.g. the root of their drive.

If this is indeed covered by Safe Browsing downloads protections, then I think we can treat this as a functional bug or feature request along the lines of "Warn users when setting too-broad default download directory". +dtrainor@ could you help triage this from a Downloads perspective?
> Additionally, if an extension downloads to an executable file, this should be checked by Safe Browsing, as it would for a normal browser download. +vakh@ to confirm.

Yes, any downloads via chrome.downloads.download() API are checked by Safe Browsing.

I also manually verified this by using the sample extension at: https://cs.chromium.org/chromium/src/chrome/common/extensions/docs/examples/extensions/download_images/ and checking that the relevant metrics are updated on chrome://histograms/SBClientDownload
Labels: -Type-Bug-Security -Restrict-View-SecurityTeam -Security_Severity-Low -Security_Impact-Stable Type-Bug
Status: Untriaged (was: Available)
Summary: User not warned when setting too-broad default download directory (was: Security: Sandbox escape via directory traversal)
Thanks Varun. Given that these have full downloads protection, I'm going to re-title this bug, remove it from the security queue, and mark it for further triage by Downloads folks.
Cc: austinknight@chromium.org
Owner: nancygao@chromium.org
+nancygao@ and +austinknight@ to help figure out if we want to show some kind of warning when the user picks an unexpectedly broad default download directory.
Status: Assigned (was: Untriaged)
Owner: xingliu@chromium.org
Let's show a warning with the text, "The default download location you selected (<location>) is not recommended.  Are you sure?"  Yes/No

I'm not quite sure when to trigger this.  We can start with any top level directory.
Also if a user tries to download into C:\Windows, that might also be too broad.
chrome.filesystem API also has a list of graylisted paths [*]. The user is warned if they grant read/write access to one of these paths.

[*] https://cs.chromium.org/chromium/src/extensions/browser/api/file_system/file_system_api.cc?rcl=070ceff1bb81f49e83ff51f0af046ab7edd59c62&l=175

Sign in to add a comment