Closed Bug 1319370 (CVE-2017-5414) Opened 4 years ago Closed 4 years ago

webkitdirectory - OS username disclosure

Categories

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

52 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla53
Tracking Status
firefox-esr45 --- unaffected
firefox51 --- wontfix
firefox52 --- verified
firefox53 --- verified
firefox54 --- verified

People

(Reporter: qab, Assigned: baku)

Details

(Keywords: csectype-disclosure, sec-moderate, Whiteboard: [post-critsmash-triage][fingerprinting][adv-main52+])

Attachments

(3 files)

Attached file 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.
*unknowingly @ #4
Based on bug 1188880, jwatt, do you know what's going on here?
Group: firefox-core-security → core-security
Component: Untriaged → DOM
Flags: needinfo?(jwatt)
Product: Firefox → Core
Flags: needinfo?(jwatt)
Baku, you were the last person to be working on this code. Can you look into this?
Flags: needinfo?(amarchesini)
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.
Flags: needinfo?(amarchesini)
Group: core-security → dom-core-security
(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?
Flags: needinfo?(amarchesini)
Priority: -- → P1
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attached patch dir.patchSplinter Review
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.
Attachment #8813613 - Flags: review?(jwatt)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #8813613 - Flags: review?(jwatt) → review+
This is not sec-high. I land the patch.
Flags: sec-bounty?
This patch landed months ago:

changeset:   324045:7477d9a20031
date:        Wed Nov 23 13:02:10 2016 +0100
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
(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?
Flags: needinfo?(amarchesini)
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
Flags: needinfo?(amarchesini)
Attachment #8813613 - Flags: approval-mozilla-beta?
Target Milestone: --- → mozilla53
Comment on attachment 8813613 [details] [diff] [review]
dir.patch

file picker fix, beta52+
Attachment #8813613 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(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
Flags: needinfo?(amarchesini)
Attached patch m-bSplinter Review
Flags: needinfo?(amarchesini)
Group: dom-core-security → core-security-release
Flagging this for manual testing, testcase & str in Comment 0.
Flags: qe-verify+
Both bugs still work 100% using Nightly 54.0a1 (2017-02-03) (64-bit)

Why is this marked fixed? What am I missing?
Flags: needinfo?(gijskruitbosch+bugs)
(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?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(amarchesini)
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
> 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.
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Flags: needinfo?(amarchesini) → needinfo?(qab)
Resolution: --- → FIXED
(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
Flags: needinfo?(qab) → needinfo?(amarchesini)
> Should I file a different report for the behavior where holding down enter

Yes, please. It's a separate bug. Thanks. And NI me.
Flags: needinfo?(amarchesini)
Flags: sec-bounty? → sec-bounty+
Whiteboard: [fingerprinting]
Summary: webkitdirectory - OS username or arbitrary local file disclosure → webkitdirectory - OS username disclosure
Changed title for more accuracy. The local file disclosure has been split out into Bug 1338637.
Whiteboard: [fingerprinting] → [post-critsmash-triage][fingerprinting]
Whiteboard: [post-critsmash-triage][fingerprinting] → [post-critsmash-triage][fingerprinting][adv-main52+]
Alias: CVE-2017-5414
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.
Group: core-security-release
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.