Issue description

VULNERABILITY DETAILS
Using a hidden input[type=file] element, in conjuncture with minimal user interaction I am able to read _ALL THE FILES IN A VICTIMS LOCAL DISK_
Please refer to the PoC for a live example.
VERSION
Chrome Version: Version 52.0.2743.116 m + stable (64-bit)
Operating System: Window 8.1 64-bit
REPRODUCTION CASE
If you look at the PoC, you will notice I ask the user to hold down the Enter button. Using some simple javascript I am then able to trick the user into giving me access to all of his/her files on local disk. Having the user to hold down the enter key for literally no more than 5 seconds is enough to read their entire computer. I dont think this is a lot to ask given the severity.
There are multiple issues with the folder upload feature on chrome:
- The file dialogue always defaults to C:
- The file dialogue can be triggered without user clicking anything
- Once the dialogue opens, all a user needs to do is press enter with no other things needed. This is different with the normal file uploader where you have to select something first.
- Most importantly. The 'webkitdirectory' feature actually crawls through all the files, irrespective of the 'accept' attribute value. In the PoC here I set accept=plain/* and I end up reading everything including images and binaries.
This is extremely dangerous for Chrome users and just to give you proof this is a bug, on firefox the file dialogue is blocked via popup blocker so its not automated as it is here.
---Suggestions to fix -------
I would suggest not always defaulting to C:/ but rather default to something like mycomputer and of course do not allow crawling of all the folders within a certain folder.
Alternatively, have the popup blocker block the file/folder choosing prompt from untrusted websites. (like firefox does)

On Windows, we could use BFFM_ENABLEOK to have the dialog start out with a disabled OK button, then start a timer from the BFFM_INITIALIZED message so that it's not enabled for a half-second or whatever timeout we think is reasonable.

I actually meant ignoring the result of the directory listing returned from the dialog if the time passed is too short (making the dialog a no-op), but not allowing the user to accept the dialog before a certain timeout also seems reasonable. We do that for extension install dialogs using a 500ms delay.

jsbell@ flushing input queue is not possible.
It seems reasonable some timeout to debounce this. Is that possible in the file input control?
I don't think there is anything that I can do in the input stack and it needs to be at the file api/input file control (which is Blink>Forms) to fix this.

This is one of our oldest open security bugs (480 days). We really need to get progress on it.
Note that Medium severity bugs are Pri-1, not 2. However, I think this may be worse than Medium. I would call it High; it works too easily. The 'gesture laundering' is quite problematic, as is the default directory (/home on Linux — ?!, and \Users\palmer on Windows).
fortunately, on Linux, the exploit (even the limited one in #2) simply crashes Chrome after grinding for a while. I take it webkitdirectory just doesn't work on Linux? The PoC works reliably on Windows.
Especially since the feature is non-standard and name-prefixed, I think we should consider removing it until we have a good story for how to make this safe. I don't consider it safe at all right now. I'm debating whether we should call it Critical severity, to be honest.
+estark for Security UX thoughts; +owencm for API design thoughts (we've been talking about this in other contexts)
Can someone please test how this behaves on Android? Thanks.

Re #59: this isn't very interesting on Android phones at least, as it requires multiple taps in precise places. Perhaps an Android tablet with an attached keyboard would behave a bit more like Desktop (due to default focus) but I expect not. Further, as far as I can tell, on Android and iOS, the picker acts as a single file picker rather than a folder picker.

A quick check of our use counters for Blink.UseCounter.Features for PrefixedDirectoryAttribute versus total page visits shows that this appears to be used by a tiny percentage of page visits, which does align with this being non-standard and prefixed (and I think supports un-shipping this for now).

We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?
For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

webkitdirectory is supported on Linux - not sure how you're determining otherwise. It's also supported by Firefox, Edge and Safari (as of Tech Preview 42 I believe); despite the prefix in the name, it was in high enough demand that other browsers implemented support.

I implemented the suggestion in #55 locally (ignore the result of the dialog if < 500ms between dialog launch and dialog commit). On Linux this seems insufficient; using the focus-switch trick, focus is shifted to to the Input element after the first repeat; on the second repeat the dialog is shown (and the time is recorded); the dialog pops up with focus on the Upload button (which depresses); the dialog then waits until the user releases the Enter key - which could be past the threshold - at which point it commits, with no way for the user to cancel. It may be a better mitigation on other platforms.
Can we re-enumerate the possible mitigations here?
(1) show the picker on key up (so repeat won't be triggered) (#37)
(2) start the picker with focus not on the accept button (platform-specific; may not be possible everywhere)
(3) start the picker with the accept button disabled; enable after some threshold (platform-specific; may not be possible everywhere. what is behavior if the Enter key is held down?) (#54)
(4) ignore the key down if the event is repeating (this only mitigates the specific repro; if focus starts on the input it wouldn't help) (#38)

Unshipping doesn't seem feasible; the usage is relatively high and it is relied upon by Google properties (e.g. Drive) and likely all the other "cloud storage" services (DropBox, etc)
Note that the default UX is so bad that most use of it is via script mapping a click on some more attractive UI to a click() call against the control, so I suspect the "key up" change wouldn't break the web, but I don't know forms/events that well.

Re #67: The PoC in #2 consistently crashes the entire browser on Linux.
Re #71: Is there a use counter for it? We should at least have that.
It sounds like you're saying gesture laundering is the only way that it works in the field?

> Re #67: The PoC in #2 consistently crashes the entire browser on Linux.
That may be Chrome trying to recurse through the entire file hierarchy of your home directory. The *feature* works, even if the naive POC does not. If you run through a modified repro by initially selecting a subdir then the enter key repetition still behaves per POC.
> Re #71: Is there a use counter for it? We should at least have that.
PrefixedDirectoryAttribute is the right one.
https://www.chromestatus.com/metrics/feature/timeline/popularity/47
No idea about the recent spike, but 0.03% of sites sounds about right. It's that it's a critical feature on those sites. (like Drive)
> It sounds like you're saying gesture laundering is the only way that it works in the field?
If "only" means "primary" and "it works" means "the feature is used" then sure. E.g. run through Drive's UI for "Upload Directory". That's pretty typical.

I suggest approaching this as follows:
(1) Land the keyup mitigation for webkitdirectory. It's not perfect because an attacker could still trick the user into clicking the upload button or pressing Enter again. But it helps a little bit, on all platforms. And there's no harm in committing a partial fix because the bug is already public.
(2) Land the keyup mitigation for <input type=file> as well. I suggest doing this separately because it seems more likely to cause compat controversy.
(3) Land platform-specific mitigations: change the default directory on Windows and/or see if we can get the timer to work as described in #54. Investigate if similar things are possible on Mac and Linux.

Re #77: I'm not sure what happened, but I don't think the fix in #76 was effective. In https://www.bayden.com/test/upload.asp, if you focus the upload button and hold down Enter, the dialog appears and auto-submits.
Similarly, the POC HTML file attached Issue 782842 still works in 65.0.3298.2.

Re #78: Ah. I'm not sure why the behavior at upload.asp is flaky, but the mitigation is completely bypassed in the POC for 782842 because the attack page simply calls .click() on the control after verifying that the user has the Enter key depressed (this scenario is alluded to in Comment 75.1).
So I don't think we're out of the woods here yet.

I have a WIP patch at https://chromium-review.googlesource.com/c/chromium/src/+/833399 for Mac+Win that disables the OK button until the user changes the selection. Haven't successfully compiled/tested the windows version yet but the mac version defeats the PoC in this bug and 782842. I *think* this might be what Safari is doing as well.
Does anyone have thoughts about this approach? It could be a little annoying from a usability perspective but at this point I think we should prioritize a reliable fix asap, even if it's a little annoying.

Re #80: The approach for Windows in the WIP looks like it /should/ work, but unfortunately it doesn't, because the Selection change message is getting sent without a user action after initialization. To workaround, we could perhaps require that 500ms or so has passed between BFFM_INITIALIZED and BFFM_SELCHANGED before enabling the button?
(Additionally, compile fails with
../../ui/shell_dialogs/select_file_dialog_win.cc(532,43): error: reinterpret_cast from 'int' to 'LPARAM' (aka 'long') is not allowed
SendMessage(window, BFFM_ENABLEOK, 0, reinterpret_cast<LPARAM>(1)); )

Re: compile fail, yeah I don't have a windows box so I'm compiling on the bots...
Is there anything to distinguish the BFFM_SELCHANGED messages before user action occurs? What's the value of the parameter in those cases? I was hoping to avoid timing stuff because it doesn't seem like half a second is enough for a user to see and process the dialog and realize that they should stop pressing the key... and the longer we go the more annoying it gets.

Re #83: The lParam on the BFFM_SELCHANGED is a PIDL (basically, a reference to the currently-selected path).
Unfortunately, I don't think there's a good way to use this approach that doesn't involve a timer. We could do something whereby we check (via GetAsyncKeyState or whatever) whether the ENTER key is down while the dialog is initializing and refuse to enable the OK button until it's not?
Alternatively, we could require an out-of-band confirmation from the user that they do indeed want to upload the files (e.g. show a Chrome UI explaining exactly what the web page is going to receive, with no default button).
Another approach would be to select a non-valid path (e.g. "My Computer") as the default, such that Windows doesn't enable the OK button. Unfortunately, it sounds like this may have broken in Windows 10. https://answers.microsoft.com/en-us/insider/forum/insider_wintp-insider_files-insiderplat_pc/problem-with-shbrowseforfolder/a263da47-36f4-433f-b86f-20ef95d60fb8

Re #83: "it doesn't seem like half a second is enough for a user to see and process the dialog and realize that they should stop pressing the key"
An important point here is that it doesn't really matter if the user stops holding the key; the OK button will be disabled because we disabled it and the user hasn't selected a folder manually. The timer is only needed to prevent non-user-initiated BFFM_SELCHANGED events that occur as Windows expands the tree in the the dialog.
struct SelectFolderDialogOptions {
const wchar_t* default_path;
bool is_upload;
+ ULONGLONG initialized_tickcount;
};
Inside if (message == BFFM_INITIALIZED) { if (options->is_upload) { we then disable the OK button and set the initialization tickcount:
options->initialized_tickcount = GetTickCount64();
SendMessage(window, BFFM_ENABLEOK, 0, 0);
Then, we reject enabling of the OK button on BFFM_SELCHANGED until a tick count expires:
SendMessage(window, BFFM_ENABLEOK, 0,
static_cast<LPARAM>(
(GetTickCount64() - options->initialized_tickcount > 1000) ? 1 : 0 ));
The downside of this approach is that if the user picks a folder in less than a second, they'll have to reselect it to enable the OK button, making it potentially appear flaky.

Oh, I see, that makes sense and sounds okay to me. (I was thinking of the proposal in #53/54.) Is there anyone with a Windows dev environment set up who can build and test elawrence's proposal in #86? I'm trying to get one set up but it will take me a few days at least. (I changed my patch in #80 to be Mac-only.)

RE #87: I've been testing this on Windows for a while. It's effective against the POC's in this bug and Issue 782842 .
With regard to Edge, yes, I believe it's using the IFileDialog with the FOS_PICKFOLDERS option as suggested by the SHBrowseForFolder documentation. However, using the IFileDialog alone isn't enough (it's what Firefox uses and Firefox is vulnerable to this attack).

Update: Eric's got a Windows fix up for review, thanks Eric!!!
So that leaves us with linux. +erg and estade: could either of you help us with this high-priority security UX bug? What we'd like to do ideally is disable the OK button on the file upload dialog until the user has changed the file/directory selection. Do you know if that's possible on Linux?

I have a patch here[1] for gtk. There's also a KDE file chooser which I didn't change.
Since there's no way to start with no selection, it disables the "open" button until the selection changes. My only misgiving is that if the default selection (first file in the list) is the one the user wants to open, it's unintuitive to have to select something else then re-select the first file. It might even mean they have to navigate to a different directory, then go back again if there's no other file there.
[1] https://chromium-review.googlesource.com/#/c/chromium/src/+/838411

estade@: I think it would be too frustrating (especially for the single file in a directory case) to wonder why the OK button isn't becoming enabled.
Could we instead enable the OK button only when the enter key is released? Or not open the dialog at all until it's released?

RE #100: To be precise, the Windows patch has exactly the same limitation as the gtk patch in #97-- the user must select a different item in the picker in order to enable the OK button. (The timer is used to avoid the problem that Windows sends "the user changed the selection" notifications erroneously during construction of the UI).
While we all agree that the "must select a different file once" behavior is a shortcoming, the alternative near-term proposal is to rip out the API entirely, which would definitely be more disruptive.
In an ideal world, we'd probably have something like what Firefox does for other dialogs (like the Save File prompt) whereby the dialog appears with the Acceptance Button disabled. The UI visually counts down for some short period (e.g. 3 seconds) so the user understands that they cannot accept right away. The problem with getting to that ideal world is that it would almost certainly require doing away with all of our platform-provided file picker UIs and rewriting our own in WebUI or Cocoa/Views, which would be a major project.

WRT #99: The patch in #76 was circumvented because it only handled the case where the dialog was shown because the user ENTER on the control itself. In an attack scenario, the attacker instead triggers the dialog by doing control.click() such that the held ENTER key flows through to the subsequent dialog.
We /could/ have code inside FileInputType::HandleDOMActivateEvent that does the equivalent of:
while GetAsyncKeyState(VK_RETURN) {
// pump events
}
...to avoid allowing a held-down ENTER key from flowing through to the platform-specific file picker dialog. The attacker would need to change their social engineering script to something like "Hit ENTER ten times" or something like that, and pop the dialog between the second and third keystroke.

c#102: I think that's better than having to select a different file. If we're concerned about the "press enter 10 times" case, then we could combine the solution in #102 with a 1s delay to enable the OK button.
I just want to avoid the scenario where users are confused why the OK button isn't working :)

> with a 1s delay to enable the OK button.
To be clear, I don't think we have the ability to enable the OK button at a specific time on Windows (and possibly other platforms).
We *can* prevent the user's selection changes from automatically enabling the OK button for a fixed period of time (that's what the patch in #96 does) but the button does not automatically become enabled when that timer expires.

We had the same issue with selection changing during ui construction on Linux, but we're able to get around that without resorting to a (hacky?) timer by instead ignoring all selection changes that come before the ui is first drawn.
If the question is whether we can switch over to just using a timer completely, I think we can --- but would that solve the problem? If you hold Enter for five seconds it will just delay the confirmation of the selection by one second.
I'm not really that opposed to forcing the user to change the selection. We could add a message within the file chooser (GTK gives us that ability) if it's really that confusing; we could add an extra button or checkbox like the "I am not a robot" captcha; etc. If this is urgent I'd just land as is and worry about UI improvements later.

Re #106: On Windows, we unfortunately get non-user-initiated selection change events both before and after the BFFM_INITIALIZED event.
> If the question is whether we can switch over to just using a
> timer completely, I think we can --- but would that solve the
> problem? If you hold Enter for five seconds it will just delay
> the confirmation of the selection by one second.
I think the hope is that the user, upon seeing the unexpected dialog, would release the Enter key. The concern noted in #83 is "How fast can we reasonably expect the user to react to the unexpected?"

Re #109: Perhaps ideally we would not allow Enter alone to submit the dialog, instead requiring that the user first manually select a folder, as this is a user-gesture that is significantly harder to fake. Unfortunately, the platform-specific dialogs do not seem to generally offer a good mechanism for requesting such behavior.

On windows I suspect you could inject a windows subclass of the dialog so that you can see mouse/key events. With that ability you could then reenable the button when some key/mouse combination occurs.

Re #112: I believe the Windows fix at https://chromium-review.googlesource.com/c/chromium/src/+/835068 to be effective, but it results in the user-experience regression that the user is forced to pick a different folder in order to submit. I believe that's a fine tradeoff in a world where the alternative on the table is to remove the Web API entirely.
If the appropriate UX powers-that-be deem that unshippably confusing/bad/etc, we could take a narrower one liner fix that prevents the dialog from appearing until the user releases the ENTER key. That protects against fewer scenarios but is even less likely to be noticed by a user.

I'm pretty swamped. For reference, the Mac-specific comment from http://crrev.com/c/835068 was
> Patch Set 8:
>
> > Patch Set 8:
> >
> > (1 comment)
> > avi@ said:
> > You're already reverting, but here are my thoughts:
> >
> > You want to enable/disable the OK button. The way you find that button is that you use -[NSView viewWithTag:] to search for the OK button, which is documented to be tagged with NSFileHandlingPanelOKButton. That's what you should be enabling/disabling.
> tapted@ said:
> I played with the POC a bit. I'm not sure disabling the button is right. If someone really wants to pass their desktop folder or some other default `favourite` it's unclear what they need to do to enable the button.
>
> Mac doesn't key-repeat like other platforms. Also buttons activate on keydown not keyup. I'm not sure this is because of key repeat. I think instead this might be because we are redispatching the event from the webcontents after it didn't preventDefault on it.
>
> That is, it ends up in command_dispatcher.mm's redispatchKeyEvent where we just do `[NSApp sendEvent:event];`. In most valid use-cases this goes to the mainMenu to activate some menu command like Cmd+L. But Enter gets redispatched too AFAIK.
>
> Maybe we can check something in redispatchKeyEvent. E.g. if `[owner_ attachedSheet]` is non-nil we probably don't want to redispatch the event.
I haven't done any experiments to see whether this is feasible yet. (if it is, and nothing else breaks, it should be a simple fix)

The CL in #116 lands "Prevent a held-down Enter key from closing the Open File dialog" for Mac. I don't think we have a good way to do exactly that for Windows, but it should be pretty easy to do "Delay the Open File dialog from opening until the user releases the ENTER key" which should provide similar protection.
Would this be preferable to the current Windows CL in #113?

Reminder that this is our oldest High severity security vulnerability (536 days), and is from an external reporter who is probably hoping to get a VRP payout. keishi, can you please make this bug a high priority, or reassign it to someone who can?
And, if that's not possible, I'll reiterate again that the feature is prefixed as well as being unsafe. Unshipping it absolutely should be an option. We can't have High severity bugs in nominally-experimental features for ~ 18 months. +awhalley FYI

Bruce, could you have someone on the Windows side cover this? The right thing seems to be preventing a default directory selection at the interface to blink. However, it also seems that the other platforms have done some whack-a-mole at OS layer, so maybe we just do that for Windows (still assuming the right solution is to prevent a default selection rather than try to race the keypress).
I talked with @palmer about severity. This feels much more medium to me. High severity is reserved for zero-interaction bypass of critical security boundary (such as silent UXSS, RCE, or sandbox escape). I appreciate that it was under-scored originally as low, but given that the user interaction is significant enough to prevent e.g. mass exploitation via a malicious ad network, I feel that medium is the right call.

#130 adds a confirmation dialog to all browsers building with toolkit_views enabled (this includes Mac). The dialog asks the user to confirm that they wanted to upload N files and which folder was selected. The dialog defaults to Cancel so tricking the user into accepting the dialog without noticing is significantly harder.
There's a few potential additional measures we could do, but given that the API usage is fairly low I'd opt for moving on.

I would like this change to spin more in Canary/Dev before making that call. The API usage is very low (<0.001% of page loads) so any crash rates or similar will take significantly longer to manifest. I'll do the merge if you want to make that call, but I think we should consider the change virtually untested in Canary until it's spun for a lot longer because of this.
If we have evidence of ongoing abuse that shifts my position here of course (or if you feel strongly about merging this change). I'm kind of shrugging my shoulders here.

Tested the issue on mac 10.13.3, win-10 and ubuntu 14.04 using chrome version #66.0.3356.0 using the html file from comment #2.
Attached screen casts for reference.
Observations on mac:
====================
On opening the PoC-Read-all-Filesv2.html file in mac 10.13.3 and pressing the enter button for 5 seconds, opened a pop up to upload the files from a directory with "upload" and "cancel" button.
Observations on Win-10:
=======================
On opening the PoC-Read-all-Filesv2.html file in win-10 and pressing the enter button for 5 seconds, intermittently opened a pop up to upload the files from a directory with "upload" and "cancel" button as per the screenshot at comment #137.
Observations on Ubuntu 14.04:
=============================
Fix is working as expected. On opening the PoC-Read-all-Filesv2.html file in ubuntu 14.04 and pressing the enter button for 5 seconds, opened a pop up to upload the files from a directory with "upload" and "cancel" button.
pbos@ - Could you please check the attached screen casts and please let us confirm the fix on OS-Win and OS-Mac.
Thanks...!!

That you're not getting the interstitial on Mac is because tapted@ has already added some mitigation for when holding Enter in the MacOS folder picker. This can be defeated by making the user press Enter repeatedly instead of holding it.
Looks like WAI on Windows, the text of the page gets changed when it brings up the (that gets instantly accepted). It takes time before you get to confirm it is because it's counting your 45k files, which is unfortunate. There's an alert in there (alert('I can read ' + this.files.length + ' files from anywhere on your pc!');) that would've been triggered if you inadvertently shared files with the site.
Attaching a more visible version that you can use for exploit testing. Click on the <input> field and only press Enter. If nothing happens by just pressing Enter repeatedly (because you cancel the confirmation dialog), then the mitigation is working. If you hit the alert in there then the dialog got accepted by just pressing Enter which isn't expected.
tapted@ also brought to my attention that Mac will accept the default button when pressing Enter regardless of where focus lies, so this has to be revisited for Mac. One suggestion is to not provide a default button (no blue button). I'll have to check with UX, but at least MacOS have some level of mitigation in place.

To append to #139 I spoke with tapted@, I think we're fine with the current level of mitigation for Mac. This is no longer a silent hold-enter trigger on any platform, any exploit is therefore significantly harder. Given that neither Windows or Mac end up passing files back on to the site by holding Enter, fix lgtm.

I find it a bit odd that the prompt asking you whether or not you want to allow a website to exploit a security vulnerability to read your files has "OK" as the default button, yet the prompt asking you whether or not you really want an extension from the Chrome Web Store has "Cancel" as the default button.

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an eligible charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************

This bug requires manual review: There is .grd file changes and we are only 31 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), josafat@(ChromeOS), abdulsyed@(Desktop)
For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot