1319370 - (CVE-2017-5414) webkitdirectory - OS username disclosure

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect, P1)

Description

[Abdulrahman Alqabandi]

Attachment: filedis.html

User Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.99 Safari/537.36

Steps to reproduce:

Tested on latest Nightly 53.0a1 (2016-11-21) (64-bit)


-----------OS username disclosure--------

The webkitdirectory on fresh install and profile (or new website where directory has never been uploaded) seems to always default to 'C:\Users\{user}\Desktop'

1. Open PoC on a fresh URL where directory uploading has never been performed (to make sure it defaults to dekstop)
2. Hold down enter 
3. Since webkitdirectory keeps going back a folder every time its used, we only need to make sure a victim keeps hitting enter for at least 2-3 upload dialogues and then we would have successfully grabbed the OS username.

------------Arbitrary File Disclosure--------

Say you go to website 'A' and you upload a directory (doesn't actually have to upload, just as long as you loaded files) that exists in the folder 'C:\Users\{user}\Documents\', if we are able to run the attached PoC on website 'A' (say XSS or if website 'A' is codepen/jsfiddle..etc) we will be able to fool the user into uploading the entire contents of the 'My Documents' folder.

Here is one scenario:

1. Victim visits a codepen link that showcases html5 webkitdirectory.
2. Victim tests this new feature by uploading/selecting a random folder within 'My documents' then forgets about it.
3. Victim receives an attackers website which embeds a codepen code that uses the PoC here.
4. Victim then knowingly exposes his/her files.

As I mentioned, this can also be done with XSS vulnerabilities or possibly even URL spoofing.


Actual results:

In both scenarios we either disclose OS username or we have access to arbitrary files on victims computer.


Expected results:

Two suggestions:

1. Prevent input.click() from bypassing popup blocker, only allow it from mouse clicks.

2. Prevent the behavior where within the Documents folder and its contents (sub folders as well) be uploaded without explicitly choosing such an action. 

For some reason #2 so far only works on Documents folder, the folder uploader seems to ignore this behavior when on C: or desktop folder and others. But if it worked for My Documents my guess is it would work on other locations.

Comment 1

[Abdulrahman Alqabandi]

*unknowingly @ #4

Comment 2

[:Gijs (he/him)]

Based on bug 1188880, jwatt, do you know what's going on here?

Comment 3

[Jonathan Watt [:jwatt]]

Baku, you were the last person to be working on this code. Can you look into this?

Comment 4

[Andrea Marchesini [:baku]]

This issue is about how we create a FilePicker and I suspect it is platform depending.
On linux, the GTK FilePicker (GTKFileChooser) has a default action. It means that, pressing 'enter', the user confirms the selection of the default selected file/directory.
We could remove the default action for GTK, windows and mac, or move the focus out of the action button.

I think we should move this bug to widget components.

Comment 5

[Andrew Overholt [:overholt]]

(In reply to Andrea Marchesini [:baku] from comment #4)
> We could remove the default action for GTK, windows and mac, or move the
> focus out of the action button.

Who's going to decide which way we go here?

Comment 6

[Andrea Marchesini [:baku]]

Attachment: dir.patch

The bug was that, we were taking always the parent directory of the last selected file/directory. In this way we were exposing the full filesystem in this test script.

Comment 7

[Andrea Marchesini [:baku]]

This is not sec-high. I land the patch.

Comment 8

[Andrea Marchesini [:baku]]

This patch landed months ago:

changeset:   324045:7477d9a20031
date:        Wed Nov 23 13:02:10 2016 +0100

Comment 9

[:Gijs (he/him)]

(In reply to Andrea Marchesini [:baku] from comment #8)
> This patch landed months ago:
> 
> changeset:   324045:7477d9a20031
> date:        Wed Nov 23 13:02:10 2016 +0100

So what branches have the fix and does it need uplift?

Comment 10

[Andrea Marchesini [:baku]]

Comment on attachment 8813613 [details] [diff] [review]
dir.patch

Approval Request Comment
[Feature/Bug causing the regression]: Entries API
[User impact if declined]: wrong last-directory in nsIFilePicker
[Is this code covered by automated tests?]: none
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: it's described in the bug.
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: this patch is already in aurora. The code just takes the directory path as last visited directory, in case the nsIFilePicker is in directory mode.
[String changes made/needed]: none

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 8813613 [details] [diff] [review]
dir.patch

file picker fix, beta52+

Comment 12

[Carsten Book [:Tomcat]]

(In reply to Julien Cristau [:jcristau] from comment #11)
> Comment on attachment 8813613 [details] [diff] [review]
> dir.patch
> 
> file picker fix, beta52+

needs rebasing for beta

applying 1319370.patch
patching file dom/html/HTMLInputElement.cpp
Hunk #1 FAILED at 479
1 out of 3 hunks FAILED -- saving rejects to file dom/html/HTMLInputElement.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh 1319370.patch

Comment 13

[Andrea Marchesini [:baku]]

Attachment: m-b

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/227c1ff7e0fa

Comment 15

[Andrei Vaida [:avaida]]

Flagging this for manual testing, testcase & str in Comment 0.

Comment 16

[Abdulrahman Alqabandi]

Both bugs still work 100% using Nightly 54.0a1 (2017-02-03) (64-bit)

Why is this marked fixed? What am I missing?

Comment 17

[:Gijs (he/him)]

(In reply to Abdulrahman Alqabandi[test] from comment #16)
> Both bugs still work 100% using Nightly 54.0a1 (2017-02-03) (64-bit)
> 
> Why is this marked fixed? What am I missing?

No idea, baku?

Comment 18

[Andrea Marchesini [:baku]]

> Both bugs still work 100% using Nightly 54.0a1 (2017-02-03) (64-bit)

I cannot reproduce this STR (from the description of the bug):

1. Open PoC on a fresh URL where directory uploading has never been performed (to make sure it defaults to dekstop)
2. Hold down enter 
3. Since webkitdirectory keeps going back a folder every time its used, we only need to make sure a victim keeps hitting enter for at least 2-3 upload dialogues and then we would have successfully grabbed the OS username.

We are not going back a folder every time. We stay on the current one.
If you see some other issues, please file a new bug.

Comment 19

[Abdulrahman Alqabandi]

(In reply to Andrea Marchesini [:baku] from comment #18)
> > Both bugs still work 100% using Nightly 54.0a1 (2017-02-03) (64-bit)
> 
> I cannot reproduce this STR (from the description of the bug):
> 
> 1. Open PoC on a fresh URL where directory uploading has never been
> performed (to make sure it defaults to dekstop)
> 2. Hold down enter 
> 3. Since webkitdirectory keeps going back a folder every time its used, we
> only need to make sure a victim keeps hitting enter for at least 2-3 upload
> dialogues and then we would have successfully grabbed the OS username.
> 
> We are not going back a folder every time. We stay on the current one.
> If you see some other issues, please file a new bug.

I think the problem I have is that there is still an arbitrary file disclosure.
Should I file a different report for the behavior where holding down enter can be used to trick someone into uploading arbitrary folder? Or more specifically, once the 'upload folder' dialog appears, a user should not be able to just hit enter and select the default/last used directory. A user must explicitly choose a folder using mouse or arrow keys

Comment 20

[Andrea Marchesini [:baku]]

> Should I file a different report for the behavior where holding down enter

Yes, please. It's a separate bug. Thanks. And NI me.

Comment 21

[Abdulrahman Alqabandi]

Changed title for more accuracy. The local file disclosure has been split out into Bug 1338637.

Comment 22

[Adrian Florinescu [:aflorinescu]]

Reproduced the bad behavior on the current Release Version: 51.0.1 20170125094131, then proceeded to verify the fix on:

Ubuntu 16.04 x64 / Mac OS X 10.12 / Windows 10 x64 on the following builds:
54.0a1 20170302030206 
53.0a2 20170302004002
52.0   20170227080736

Based on the above marking this bug as verified and updating all tracking flags accordingly.