Bug 1319370 (CVE-2017-5414)

webkitdirectory - OS username disclosure

VERIFIED FIXED in Firefox 52

Status

()

P1
normal
VERIFIED FIXED
2 years ago
a year ago

People

(Reporter: qab, Assigned: baku)

Tracking

({csectype-disclosure, sec-moderate})

52 Branch
mozilla53
csectype-disclosure, sec-moderate
Points:
---
Bug Flags:
sec-bounty +
qe-verify +

Firefox Tracking Flags

(firefox-esr45 unaffected, firefox51 wontfix, firefox52 verified, firefox53 verified, firefox54 verified)

Details

(Whiteboard: [post-critsmash-triage][fingerprinting][adv-main52+])

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
strtestcase
Created attachment 8813100 [details]
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.
(Reporter)

Comment 1

2 years ago
*unknowingly @ #4

Comment 2

2 years ago
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

Updated

2 years ago
Flags: needinfo?(jwatt)
Baku, you were the last person to be working on this code. Can you look into this?
Flags: needinfo?(amarchesini)
(Assignee)

Comment 4

2 years ago
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)

Updated

2 years ago
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
(Assignee)

Comment 6

2 years ago
Created attachment 8813613 [details] [diff] [review]
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.
Attachment #8813613 - Flags: review?(jwatt)
(Assignee)

Updated

2 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true

Updated

2 years ago
Attachment #8813613 - Flags: review?(jwatt) → review+
(Assignee)

Comment 7

2 years ago
This is not sec-high. I land the patch.
Keywords: sec-moderate
Keywords: csectype-disclosure
Flags: sec-bounty?
(Assignee)

Comment 8

2 years ago
This patch landed months ago:

changeset:   324045:7477d9a20031
date:        Wed Nov 23 13:02:10 2016 +0100
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED

Comment 9

2 years ago
(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)
(Assignee)

Comment 10

2 years ago
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?

Updated

2 years ago
status-firefox51: --- → wontfix
status-firefox52: --- → affected
status-firefox53: --- → fixed
status-firefox54: --- → fixed
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)
(Assignee)

Comment 13

2 years ago
Created attachment 8832478 [details] [diff] [review]
m-b
Flags: needinfo?(amarchesini)
Group: dom-core-security → core-security-release
Flagging this for manual testing, testcase & str in Comment 0.
Flags: qe-verify+
(Reporter)

Comment 16

2 years ago
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)

Comment 17

2 years ago
(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 → ---
(Assignee)

Comment 18

2 years ago
> 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
Last Resolved: 2 years ago2 years ago
Flags: needinfo?(amarchesini) → needinfo?(qab)
Resolution: --- → FIXED
(Reporter)

Comment 19

2 years ago
(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)
(Assignee)

Comment 20

2 years ago
> 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)
status-firefox-esr45: --- → unaffected
Flags: sec-bounty? → sec-bounty+
Whiteboard: [fingerprinting]
(Reporter)

Updated

2 years ago
Summary: webkitdirectory - OS username or arbitrary local file disclosure → webkitdirectory - OS username disclosure
(Reporter)

Comment 21

2 years ago
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.
Status: RESOLVED → VERIFIED
status-firefox52: fixed → verified
status-firefox53: fixed → verified
status-firefox54: fixed → verified
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.