Closed Bug 1249522 (CVE-2016-5279) Opened 4 years ago Closed 4 years ago

Full local path of files is available to web pages after drag and drop

Categories

(Core :: DOM: Drag & Drop, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
mozilla49
Tracking Status
firefox49 --- verified

People

(Reporter: rafael, Assigned: enndeakin)

References

Details

(Keywords: csectype-disclosure, sec-moderate, Whiteboard: [adv-main49+] btpp-fixlater)

Attachments

(1 file, 2 obsolete files)

+++ This bug was initially created as a clone of Bug #1226977 +++

1. Visit <https://ebenda.org/2016/file-path/display.html>.
2. Drag and drop a file from Windows Explorer or (GNOME/Ubuntu) Files (Nautilus) into the web page.
3. You will see the full path of the dragged path.

The full path will be available via dataTransfer.getData("text/x-moz-url") on Windows and GNU/Linux (and probably OS X).

It will also be available via various other types when dragged from Nautilus.

The "text/x-moz-url" content is completely synthesized by Firefox, whereas Nautilus really sends ['x-special/gnome-icon-list', 'text/uri-list', 'UTF8_STRING', 'COMPOUND_TEXT', 'TEXT', 'STRING', 'text/plain;charset=utf-8', 'text/plain']) containing the file URL. Firefox will have, for better or worse, to disregard these types in this case as to conform to the platform's convetion (Chrome seems to disregard all other types if Files are provided during drag and drop).

It is questionable whether this path should be available to web content for privacy reasons. It also allows for further exploitation ("Disclosure of OS username" is the most easy) and can help to exploit other bugs (as shown in bug 1226977 comment 10). For the input element, it was considered a security bug (<https://www.mozilla.org/en-US/security/advisories/mfsa2013-43/>).

Existing web content might depend on this (a possible use case would be an internal CMS storing paths of images dragged and dropped from a shared network drive to track and automatically update them on changes). Chrome does not expose the full path of dropped files to web pages in either case (unsure about other browsers implementing dragging and dropping of files), so this is not really web-compatible anyhow, and I guess Firefox should not expose the paths either.
I can work on it after, but it is a different issue.
Flags: needinfo?(enndeakin)
(In reply to Rafael Gieschke from comment #0)
> Chrome does not expose the full path of dropped files to web pages in either case
> (unsure about other browsers implementing dragging and dropping of files),
> so this is not really web-compatible anyhow, and I guess Firefox should not
> expose the paths either.

We long ago stopped revealing the full file path as part of the file upload control. This is analogous and we should do the same kind of stripping.
Group: core-security → dom-core-security
Status: UNCONFIRMED → NEW
Ever confirmed: true
Whiteboard: [btpp-followup-2016-02-26] → btpp-fixlater
Yes. I'll mark the other one a duplicate as this one has more info. Affects Windows and Linux but does not affect Mac.
Assignee: nobody → enndeakin
Status: NEW → ASSIGNED
Duplicate of this bug: 666387
So there are two options here:

1. When a file is being dropped, don't expose the non-file data.
2. When a file is being dropped, throw an error when unprivileged code tries to get non-file data. 

I suspect some chrome code may rely on the non-file data, so the latter would be less likely to cause regressions.
I forget what the web-exposed APIs look like for drag'n'drop. But we should definitely expose both the file and the filename (but not the whole path) to the webpage.

If there's chrome code which relies using the same APIs as the ones we expose to the web, but that still need the full path+filename, then we can put logic in the C++ code which checks if the caller is chrome or a webpage and adjust the returned value based on that.
The filename is already exposed via dataTransfer.files[x].name

The issue here is the OS is providing a text or url alternative to the file that includes the full path. The proposal here is to either remove these or throw an error when trying to retrieve them. The files portion would be unaffected.
Ah! Thanks!

Removing them is likely safer from a webcompat point of view. There's always risk that a page tries to get multiple different formats and end up breaking if one of them throw an exception. Especially if there's API for enumerating different formats.

But I don't know for sure if throwing will be a problem, so I don't feel strongly either way.
Attached patch Remove non-file types (obsolete) — Splinter Review
This patch removes the non-file types when types or getData (or the moz variants) are called from a non-privileged context. Chrome callers are left alone.

A test is added which tests this for clipboard use.
Attached patch Remove non-file types (obsolete) — Splinter Review
I changed all type getting code to share the same path. This patch fixes a test that failed from the previous patch.
Attachment #8743972 - Flags: review?(bugs)
Comment on attachment 8743972 [details] [diff] [review]
Remove non-file types

+  yield new Promise((resolve, reject) => {
+    searchbar.addEventListener("paste", function copyEvent(event) {
+      searchbar.removeEventListener("copy", copyEvent, true);
You're adding listener for paste, but removing listener for copy?

>+    if (addFile) {
>+      // If this is a content caller, and a file is in the data transfer, remove
>+      // the non-file types. This prevents alternate text forms of the file
>+      // from being returned.
>+      if (addFile && !(nsContentUtils::GetCurrentJSContext() &&
>+                       nsContentUtils::IsCallerChrome())) {
So you're checking addFile already above, so no need to do it again.
And I would use !LegacyIsCallerChromeOrNativeCode() here.
It changes the behavior a bit though, since native caller wouldn't be handled as content JS, like your code does here.

+  // If this is a content caller, and a file is in the data transfer, only
+  // return the file type.
+  if (!format.EqualsLiteral(kFileMime) &&
+      !nsContentUtils::IsSystemPrincipal(aSubjectPrincipal)) {
+    uint32_t count = item.Length();
+    for (uint32_t i = 0; i < count; i++) {
+      if (item[i].mFormat.EqualsLiteral(kFileMime)) {
+        return NS_OK;
+      }
+    }
+  }
So what should happen if content tries to access "Files" and not application/x-moz-file?
Attachment #8743972 - Flags: review?(bugs) → review-
> So what should happen if content tries to access "Files" and not
> application/x-moz-file?

Nothing special. The "Files" special name only applies to .types
Attachment #8736425 - Attachment is obsolete: true
Attachment #8743972 - Attachment is obsolete: true
Attachment #8747232 - Flags: review?(bugs)
Attachment #8747232 - Flags: review?(bugs) → review+
I filed bug 1270277 on the cause of the Android failure.
Flags: needinfo?(enndeakin)
https://hg.mozilla.org/mozilla-central/rev/d4e621e02edc
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
Group: core-security-release
Group: dom-core-security
Whiteboard: btpp-fixlater → [adv-main49+] btpp-fixlater
Alias: CVE-2016-5279
Reproduced the issue on Nightly 2016-03-01.
Verified fixed FX 49.0 on Win 7, Ubuntu 14.04, OS X 10.11.
Status: RESOLVED → VERIFIED
Depends on: 1273371
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.