Open Bug 907707 Opened 11 years ago Updated 2 years ago

Security issues related to users making directories available to a page via <input type=file directory> or drag-and-drop

Categories

(Core :: DOM: Forms, defect, P3)

Product:
Core
Component:
DOM: Forms
Type:
defect

Tracking

()

People

(Reporter: jwatt, Unassigned)

References

(Depends on 2 open bugs)

Details

(Keywords: privacy, sec-want) 

As pointed out by mats in bug 907428 comment 8, we should consider restricting the directories that a user can select using <input type=file>. Specifically it would be good to avoid revealing username(s) (the username being in paths of the attached files if / or /home or /tmp are picked, for example).
An alternative to imposing restrictions would be to anonymise the paths.
For example, if you pick /home/alice/file1 and /home/bob/file2
we would send user1/file1 and user2/file2
Not allowing paths under ~ seems extremely restrictive.  I'd expect one of the primary use cases of this is uploading a directory of photos which is probably under ~ somewhere.
Restricting directory picking sounds like a bad idea. The annoyance/security ratio is way too high. My username can be revealed in other ways (eg. it might appear in another file name that we did not think about) and if I have a good reason to uplead my entire $HOME (eg. backup/syncing), I will not be able to do so.

I think a solution like comment 1 is probably better. Except that it might create some problems in a syncing scenario where /home/user1/ would actually not mean much because not the real username.

Another solution would be to let the user know that he/she is leaking some information to the website. But, my feeling is that if you try to upload things as sensitive as / or /home, you might trust the website enough already because the website will have access to information likely more sensitive than your username.
(In reply to Kyle Huey [:khuey] (khuey@mozilla.com) from comment #2)
> Not allowing paths under ~ seems extremely restrictive.  I'd expect one of
> the primary use cases of this is uploading a directory of photos which is
> probably under ~ somewhere.

There'd be no need to put restrictions on picking a directory _under_ your home directory. If my file system looks like this:

  home
    jwatt
      Pictures
        foo
          bar
          baz

and I pick 'Pictures', two files will be exposed, either with the paths "Pictures/foo/bar" and "Pictures/foo/baz", or else "foo/bar" and "foo/baz". (We've not decided if the name of the directory that is picked should be included in the paths or not.) In neither of these cases would "jwatt" be in the paths, so there would be no restriction on picking files _under_ your home directory.
I'm also leaning to the "no restriction" side of the argument FWIW.
FWIW, I tested <input type=file multiple> in the native file pickers on
OSX/Win7/Linux and it seems it's not even possible to select two files
from different directories.  That limits the problem to directory selection
then (assuming we only intend to send relative paths).

I agree that if you select and upload your $HOME directory then you are in
much much deeper trouble than revealing your user name.  I think we need
some measures to avoid doing that by mistake.

Does the <input> tell you how many files will be uploaded (recursively)
when a directory is selected?  If it looks something like:
  [Browse...] jwatt (1000 files)
then that might be a good warning.  I doubt that's enough though.

Personally, I don't think we should allow uploading the entire $HOME
directory at all.  There's no way an average user understands the
security implications of doing that.
We're still waiting for shorlander's UX designs in bug 846931. CC'ing him so that he can take account of the conversation going on here.
(In reply to Mats Palmgren (:mats) from comment #1)
> An alternative to imposing restrictions would be to anonymise the paths.
> For example, if you pick /home/alice/file1 and /home/bob/file2
> we would send user1/file1 and user2/file2


if the user picks those two files, we are going to submit "file1" and "file2"

we only expose directory names of directories that the user explicitly pick.

I think this bug is WONTFIX. At worst we could put up a warning if the user try to select the root directory or something that contains other password files.
OK, then I misunderstood the feature (I thought you sent the relative paths
to a common ancestor to avoid name collisions).  I have no concerns about
exposing the user name when the user picks $HOME or a prefix thereof.

As I said, I think the user is in much much deeper trouble in that case.
Uploading your private encryption keys, bitcoin wallet, firefox profile,
etc etc to a server of any kind, even if you believe you can trust it,
is a really bad idea.  The consequences can be so disastrously bad that
I think we should actively prevent it.
Keywords: privacy
Keywords: sec-want
Blocks: 846931
No longer blocks: 894840
Here's another powerful feature that needs security review. We're about to allow the upload of entire directories and all their contents, potentially including hidden folders and other items that may not be obvious to the user. What kind of restrictions (if any) should we put here? At a minimum, should this be https-only?
Flags: needinfo?(rlb)
Flags: needinfo?(dveditz)
I don't want to do https-only unless other browsers do too.
(In reply to Jet Villegas (:jet) from comment #10)
> We're about to allow the upload of entire directories and all their contents,
> potentially including hidden folders and other items that may not be obvious
> to the user. What kind of restrictions (if any) should we put here?

Where are we doing this work? The conversation here has covered various ways to do it, and the UI design referenced in comment 7 is quite old and makes me wonder if there's something more recent you're working from.

Several comments above talk about sending relative paths, while the WHATWG HTML spec seems pretty clear that paths are not to be sent even if two files have the same leaf name. Is the spec going to change? Sounds like we already violated that in bug 907428.

Are we implmenting the "directory webkitdirectory" attributes that Chrome seems to support? Is that specified or documented anywhere? Or is this just an option that appears magically through the use of the "multiple" attribute (as implied by the old UI wireframe in bug 846931)?

Will users have the ability to observe/inspect the list of files--or have us warn them how many files are selected and whether or not any hidden files are part of the selection--BEFORE they are added to the page? It's nice that users can inspect them sitting in the form before the user clicks submit, but since the page itself can trigger submit() at any time that may not be good enough.

How does selecting a directory mix with an accept attribute that is trying to limit selections to images, audio, video, etc ? Do we send the whole directory regardless, or do we only send files of the matching type? What if there are multiple types specified?
Flags: needinfo?(dveditz) → needinfo?(bugs)
Blocks: 1164310
(In reply to Daniel Veditz [:dveditz] from comment #12)
> Where are we doing this work?

To avoid confusion with the previous work I've spun off bug 1164310.

> The conversation here has covered various ways
> to do it, and the UI design referenced in comment 7 is quite old and makes
> me wonder if there's something more recent you're working from.

We're specifically looking at implementing:

http://internetexplorer.github.io/directory-upload/proposal.html

My main concern is the second paragraph of comment 9 in this bug, and to have the security groups input on that.

> Several comments above talk about sending relative paths, while the WHATWG
> HTML spec seems pretty clear that paths are not to be sent even if two files
> have the same leaf name. Is the spec going to change? Sounds like we already
> violated that in bug 907428.

That wouldn't be relevant for the work for bug 1164310. Maybe we should fork this security review bug so that there's one sg bug corresponding to bug 846931 and another corresponding to bug 1164310, but the Summary of this bug seems general enough to cover both. (Although maybe we need a more general sg bug for this stuff?)

> Are we implmenting the "directory webkitdirectory" attributes that Chrome
> seems to support? Is that specified or documented anywhere? Or is this just
> an option that appears magically through the use of the "multiple" attribute
> (as implied by the old UI wireframe in bug 846931)?

According to MS's proposal (and what those of us in the Moz discussions on the proposal agree with) a 'directory' attribute would be required. The behavior would be different to Chrome's current behavior though.

> Will users have the ability to observe/inspect the list of files--or have us
> warn them how many files are selected and whether or not any hidden files
> are part of the selection--BEFORE they are added to the page? It's nice that
> users can inspect them sitting in the form before the user clicks submit,
> but since the page itself can trigger submit() at any time that may not be
> good enough.

I'd like to hear the security group's thoughts on that. (And UX's, although their thoughts probably depend on whether you think we need to do something like that.) It may be possible to insert such a stage without breaking spec behavior from the page's POV.

> How does selecting a directory mix with an accept attribute that is trying
> to limit selections to images, audio, video, etc ? Do we send the whole
> directory regardless, or do we only send files of the matching type? What if
> there are multiple types specified?

The basic idea is that the page can use script to asynchronously walk the directory. It possibly makes sense to limit what the page sees to only Directory objects and files matching the 'accept' limitations, but I don't think there is currently any such restriction.
Flags: needinfo?(bugs)
Note that the new proposal *does* include directory names when files are being uploaded. But my understanding is that only the name of the directory that the user picked, as well as the names of any subdirectories under it.

Likewise the name of the picked directory, and the the names of any files and directories in that directory is exposed through the JS API.

This is similar to how we expose the filename of the file that the user picked. Both when uploading said file, and through the JS API.

But the names of any parent directories of any picked file or directory is never included. Neither in the JS API, nor in any form submissions.
Have the security team had any more thoughts on this bug? This is a Q2 goal and, although the implementation only needs to be functional in Q2, I'd like it to also be shippable which requires decisions to be made regarding this bug.
Flags: needinfo?(dveditz)
I have some questions about each OS file/directory picker we consider using:

* Can I see what's in the directory?
* Can I choose to upload a single file, or several files, instead of a directory?
* Is it clear that I am uploading a directory?
* If I double-click a directory, is it expanded or treated as my final selection?

It's possible that we'll decide the OS directory pickers are unsuitable for untrusted apps and want to build our own.

After selecting a directory, I'd like to see an editable summary of what's in it:

    [x] 7000 audio files
    [x] 5 videos
    [ ] 20 playlists
    [ ] 5 text files

           [[Upload 7005 files from ~/Music]]

Is "Upload" the verb we'll use on button labels? It makes the security question clear, but might scare people away from granting JS access to a 10GB music directory.

Restricting this feature to https sounds good. Even though <input type=file multiple> technically allows a lot of the same security risks, this feature will start an era of broad grants to tools such as space-use analyzers and duplicate-file finders. Mistaken grants will also become more likely. When I make a mistake, I want to at least know who got access.

Will I be able to...

* Grant permanent ~/Music/ access to my favorite radio, library, and rhythm game?
* Drag a directory from Finder to a page?
* Drag a directory from a page to Finder? (Better than downloading a zip!)
* Grant direct write access?
For now I'll leave the https question to rbarnes, but I tend to agree w/Jonas in comment 11 to the extent that I don't want to become "the browser that can't". Still, I would hope we can convince MS and Google to do it that way because this enables bulk uploads on a scale far greater than currently feasible with a multiple file input element, and likely to inspire new kinds of apps (e.g. backup services, photo processing) that will encourage file sharing at a much larger scale and potentially of more sensitive content.

The proposed chooseDirectory() method, if we're doing it, needs to be limited to firing only during a non-synthetic event handler to prevent abuse.

The spec implies through its example, but doesn't actually say anywhere, that if you picked /Users/myname as the directory you'd get myname/file1, myname/file2 .... It would equally match the spec if we only passed filenames relative to the chosen directory, file1, file2...  That interpretations, however, means that the example shows a user who has chosen a directory that has a docs subdirectory but was otherwise empty. The spec should be clarified so implementations are consistent, but it doesn't seem terrible or a surprise to a user to send the name that they just clicked on (as long as we don't send the names of parent directories). that is, comment 14 sounds fine.

I'm reasonably happy with the directory picker on Mac -- when you pick a directory you can see all the (non-hidden) files in it. When I've encountered the directory picker on Windows--where all our users live--I can only see the directory names and have no idea what's in them. (I'm basing this on my experience doing things like changing the default download directory from our general preferences tab about:preferences#general ). That windows experience is unsafe for this.

I'm also concerned that users will upload an unknown number of hidden files. Do we have the ability to override the visibility settings on the picker dialog? For some services (like backups) you want to send those files, but doing so implicitly makes it far too easy to lull users into making horrible mistakes. I'm sure this worry is what led to the summary on this bug, "Consider restricting the directories that a user can select...". Blacklisting is usually the wrong solution, and in any case the most obvious directory to blacklist (~) is also likely to be a desired choice for some use cases for this feature.

What is the proposal for handling hidden files?
 - send them anyway
 - don't ever send them
 - tell the user you're sending them
 - give the user the choice to send them or not
 - something else?

Apart from the "restrict to https" question, most of the security pitfalls are going to involve the usability of the feature. There's no UX design here so it's hard to evaluate.
Flags: needinfo?(dveditz)
To hit the obvious question: I don't think this needs to be HTTPS, since (1) we haven't set a cutoff for new HTTP features, and (2) this can mostly be emulated.

I'm pretty much with with Jonas on this with regard to whether to limit parent directories, so maybe this bug needs to be renamed and WONTFIX'ed.

My main concern, as Dan's is avoiding user surprise.  If a user accidentally picks a directory that has 7M files in it, that seems like they're probably going to have a bad time.  But maybe that can be addressed by the web app.  Hidden files/directories are a bigger concern.  I would propose that we only send things that are visible in the file picker -- since those are the ones the user chose.  We shouldn't even let the JS know that there were hidden files around.  It will be an imperfect clone, but that seems livable.

This API design also seems pretty un-aesthetic.  It seems like you could just add the subordinate files into the overall list of input.files, and add path attribute that would tell you the directory structure.
Flags: needinfo?(rlb)
The Google Chrome security team is inclined to restrict this feature to https:
https://twitter.com/jruderman/status/601131553775816707
Blocks: 1188880
(In reply to Jesse Ruderman from comment #16)
> I have some questions about each OS file/directory picker we consider using:

For some background, we only support directory picking on desktop since other platforms don't have directory pickers. If script tries to open a directory picker on those platforms it HTMLInputElement::ChooseDirectory  will open a file picker instead.

> * Can I see what's in the directory?

On OS X the native picker shows you the contents of the directory, on Windows it doesn't.

> * Can I choose to upload a single file, or several files, instead of a
> directory?

When <input type=file directory> is displayed on desktop, it displays a "Choose Files..." and "Choose Folder..." button. Selecting the former gives you the current file picker behavior we've had since forever. Picking the latter opens a native directory picker that currently only allows you to pick a single directory. However, drag and drop allows you to drop a combination of file(s) and directory(s). For example, if your OS allows you to search your filesystem and get back a list of files and directories matching a certain criteria, you could drag and drop those items onto the <input> and the script would be given an array of Fire and Directory items that it could access.

> * Is it clear that I am uploading a directory?

When you pick a directory you give script access to a Directory object which it can walk to get File objects. Uploading only comes into it if the page then decides to upload content via XHR or something, or the <input>'s form is submitted. Whether the consequences of the pick are sufficiently clear to the user I think is a matter for the sg to discuss.

> * If I double-click a directory, is it expanded or treated as my final
> selection?

On OS X it is a final selection, on Windows it opens the directory.

> It's possible that we'll decide the OS directory pickers are unsuitable for
> untrusted apps and want to build our own.

For now B2G doesn't support directory picking.

> After selecting a directory, I'd like to see an editable summary of what's
> in it:
> 
>     [x] 7000 audio files
>     [x] 5 videos
>     [ ] 20 playlists
>     [ ] 5 text files
> 
>            [[Upload 7005 files from ~/Music]]

This would need to be a second modal dialog window blocking the 'change' event that fires to notify the page that the user has picked something, at which point the page has access to the picked Directory. I'm not sure how workable this is given that I/O can be slow. It might take forever to be able to show this checkbox list, since that would require traversing the entire tree first. Blocking indefinitely could result in a terrible user experience. The current directory picking implementation is designed specifically not to block so that the page can give the user prompt feedback that their pick action is actually doing something and being processed.

> Is "Upload" the verb we'll use on button labels? It makes the security
> question clear, but might scare people away from granting JS access to a
> 10GB music directory.

No, it's not about upload. It's about choosing or opening, or more specifically about giving the page the ability to read (not write) the contents of the directory. Whether the contents are then uploaded depends on whether the page chooses to do that.

> Restricting this feature to https sounds good. Even though <input type=file
> multiple> technically allows a lot of the same security risks, this feature
> will start an era of broad grants to tools such as space-use analyzers and
> duplicate-file finders. Mistaken grants will also become more likely. When I
> make a mistake, I want to at least know who got access.
> 
> Will I be able to...
> 
> * Grant permanent ~/Music/ access to my favorite radio, library, and rhythm
> game?

No. Just like when you grant access to a file via file picking the page will need to re-request it if the page is refreshed, so it is for directory picking.

> * Drag a directory from Finder to a page?

Yes. And a combination of files and directories, possibly from assorted locations in the filesystem due to a search.

> * Drag a directory from a page to Finder? (Better than downloading a zip!)

That's not part of the work that's being done here.

> * Grant direct write access?

Also not part of the work being done here.
(In reply to Daniel Veditz [:dveditz] from comment #17)
> For now I'll leave the https question to rbarnes, but I tend to agree
> w/Jonas in comment 11 to the extent that I don't want to become "the browser
> that can't".

We can always relax the restriction later. I'd hope that Let’s Encrypt makes this a realistic restriction to impose.

> The proposed chooseDirectory() method, if we're doing it, needs to be
> limited to firing only during a non-synthetic event handler to prevent abuse.

Hmm, .chooseDirectory() is supposed to be to directory picking of what .click() is to file picking. .click() is not restricted to being called under an event, let alone a non-synthetic event. (The reason for chooseDirectory existing at all is because some platforms don't support a picker that can select a combination of files and directories, so script needs to specify which of the two native pickers it wants to have brought up.)

> When I've
> encountered the directory picker on Windows--where all our users live--I can
> only see the directory names and have no idea what's in them. That
> windows experience is unsafe for this.

It wouldn't make sense to ship an implementation for OS X but not Windows. Are you saying that we need to create our own directory picker for Windows? Probably that means creating multiple different pickers for different Windows versions and themes. I'm not sure how practical that is.

> I'm also concerned that users will upload an unknown number of hidden files.
> Do we have the ability to override the visibility settings on the picker
> dialog?

We have some control over filtering in the picker for file picking, but not sure offhand about directory picking. Note however even if that is supported the filtering in the native pickers would only filter out certain directories from being *directly* selected by the user during the pick. It would have no affect on what's exposed to the page as it traverses the picked items. However, we could make Directory::GetFilesAndDirectories filter the list that it returns.

> What is the proposal for handling hidden files?
>  - send them anyway
>  - don't ever send them
>  - tell the user you're sending them
>  - give the user the choice to send them or not
>  - something else?

What's important is not just sending. More generally the consideration is what's exposed to the page (script) immediately after a picker is closed.

A prompt that does not require traversing the directory tree (see the I/O comment in comment 20) seems workable, depending on whether UX agrees it's okay. Filtering out hidden files/directories without prompting the user would restrict the usefulness of the feature, but any of these four options would work for the initial implementation that we ship from my perspective as long as SG and UX agree on something.

> Apart from the "restrict to https" question, most of the security pitfalls
> are going to involve the usability of the feature. There's no UX design here
> so it's hard to evaluate.

I think the best thing to evaluate against is the implementation that is now enabled in Nightly.
(In reply to Richard Barnes [:rbarnes] from comment #18)
> To hit the obvious question: I don't think this needs to be HTTPS, since (1)
> we haven't set a cutoff for new HTTP features, and (2) this can mostly be
> emulated.

Hmm, I guess making it HTTPS only would be pretty hard since using FileReader a script can get the contents of files, and at that point we'd need a way to taint strings as being security sensitive and prevent them being used all over the place (e.g. in the query string of a URL).

> I'm pretty much with with Jonas on this with regard to whether to limit
> parent directories, so maybe this bug needs to be renamed and WONTFIX'ed.
> 
> My main concern, as Dan's is avoiding user surprise.  If a user accidentally
> picks a directory that has 7M files in it, that seems like they're probably
> going to have a bad time.  But maybe that can be addressed by the web app. 

You seem to have in mind form submission here. In general, pages would be expected to incrementally upload using XHR.

> Hidden files/directories are a bigger concern.  I would propose that we only
> send things that are visible in the file picker -- since those are the ones
> the user chose.  We shouldn't even let the JS know that there were hidden
> files around.  It will be an imperfect clone, but that seems livable.

The clone is imperfect anyway due to us ignoring symlinks. Admittedly ignoring hidden files and directories is a bigger deal, but for our initial shipping implementation that seems acceptable to me.

> This API design also seems pretty un-aesthetic.  It seems like you could
> just add the subordinate files into the overall list of input.files, and add
> path attribute that would tell you the directory structure.

I/O can be very slow. The API was designed to try and avoid long pauses between when the user makes their selection and when the page is notified about it. Flattening the tree into input.files would mean traversing the entire tree before notifying the page which would significantly increase the risk of there being a long pause between the user making their selection and the page being able to acknowledge it.
Another suggestion I'll repeat from bug 1207114 comment 5: would relabeling the "Open"/"Choose" button in the native directory picker to "Upload" make people happier? While not technically what the user is doing at the point they make a directory selection, it makes clearer to the user that this is a natural consequence of their action and that the contents of the directory could end up being uploaded.
FWIW when using 'webkitdirectory' in Chrome on Windows the picker both lists and allows you to pick hidden folders and directories those that start with "." like '.ssh'. It also includes hidden files and directories that are under any picked directory in the FileList that it makes available to the webpage. Chrome on OS X is slightly different only in that it doesn't show directories beginning with "." in the picker, but I think this is just because there native pickers have NSSavePanel::showsHiddenFiles set to NO by default; any files/directories beginning with "." under a picked directory are still included in the FileList.

Flash based pickers also do not currently seem to have any restrictions.
It doesn't just seem like an issue of "visible". When you have a 5-deep tree, you likely
have no clue what's in the deeper branches, whether it's nominally "visible" or not
My broader point is that directory picking exists on the web now in all browsers via JS libs that abstract 'webkitdirectory' and Flash, and neither seem to restrict or filter out what a user can select.
(In reply to Jonathan Watt [:jwatt] from comment #26)
> My broader point is that directory picking exists on the web now in all
> browsers via JS libs that abstract 'webkitdirectory' and Flash, and neither
> seem to restrict or filter out what a user can select.

As I said in e-mail, I'm not sure how what's in Flash is really relevant here.
Part of why we are removing Flash is that we have judged that it's bad for
user security.
Summary: Consider restricting the directories that a user can select using <input type=file> → Security issues related to users making directories available to a page via <input type=file directory> or drag-and-drop
Flash is relevant because it has enabled directory picking in non-Chrome browsers for years now, and removing it means breaking sites that use it for directory picking in Firefox unless we provide an alternative. We consider that this one area where we need to provide an alternative to proceed with disabling Flash.

Our reasons for removing Flash are less to do with its features being bad for user security, and a lot more to do with its its propensity for frequent, unintended security holes being bad for user security.
(In reply to Jonathan Watt [:jwatt] from comment #28)
> Flash is relevant because it has enabled directory picking in non-Chrome
> browsers for years now, and removing it means breaking sites that use it for
> directory picking in Firefox unless we provide an alternative. We consider
> that this one area where we need to provide an alternative to proceed with
> disabling Flash.

Yes.


> Our reasons for removing Flash are less to do with its features being bad
> for user security, and a lot more to do with its its propensity for
> frequent, unintended security holes being bad for user security.

There are a number of reasons.
(In reply to Jonathan Watt [:jwatt] from comment #22)
> Hmm, I guess making it HTTPS only would be pretty hard since using
> FileReader a script can get the contents of files, and at that point we'd
> need a way to taint strings as being security sensitive and prevent them
> being used all over the place (e.g. in the query string of a URL).

"What if a site is careless with your data" is not normally part of the decision to make a feature HTTPS-only.
Depends on: 1212239
Depends on: 1209924
Depends on: 1235229
I'd very much like to have a "You're about to upload an entire directory to [website]" warning, with 'ok' or 'cancel'.  File picker and directory picker look the same to me, and I'd be worried that I'd not notice that a directory was being sent.
As several people here have mentioned, I don't think restricting what can be uploaded is a good idea. The current implementation excludes dotfiles, which for example is restricting me from uploading a git folder with history and project-specific dotfiles.

If I select a folder, it's my responsibilty to know what's in there, be it "hidden" or not.
Another issue worth being aware of:

https://github.com/WICG/directory-upload/issues/38
Assignee: jwatt → nobody
Priority: -- → P3
Component: DOM: Core & HTML → DOM: Forms
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.