1249522 - (CVE-2016-5279) Full local path of files is available to web pages after drag and drop

Status: VERIFIED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Rafael Gieschke]

+++ 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.

Comment 1

[Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo]

Will you work on this as part of https://bugzilla.mozilla.org/show_bug.cgi?id=1226977?

Comment 2

[Neil Deakin]

I can work on it after, but it is a different issue.

Comment 3

[Daniel Veditz [:dveditz]]

(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.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Is this a dupe of bug 666387?

Comment 5

[Neil Deakin]

Yes. I'll mark the other one a duplicate as this one has more info. Affects Windows and Linux but does not affect Mac.

Comment 7

[Neil Deakin]

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.

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 9

[Neil Deakin]

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.

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 11

[Neil Deakin]

Attachment: Remove non-file types

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.

Comment 12

[Neil Deakin]

Attachment: Remove non-file types

I changed all type getting code to share the same path. This patch fixes a test that failed from the previous patch.

Comment 13

[Olli Pettay [:smaug][[e-mail]]]

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?

Comment 14

[Neil Deakin]

> 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

Comment 15

[Neil Deakin]

Attachment: Remove non-file types, version 3

Comment 16

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/751ca24ae2c210a4491c56c2c8e6d71a7112a62e
Bug 1249522, when a file is present, only specify file type, r=smaug

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for android m(16) bustage: 
https://treeherder.mozilla.org/logviewer.html#?job_id=27226220&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6d98b750214

Comment 18

[Neil Deakin]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a9e61fa474ce

Comment 19

[Neil Deakin]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=32eaea8dd0c8

Comment 20

[Neil Deakin]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c8e9da7f7a94

Comment 21

[Neil Deakin]

I filed bug 1270277 on the cause of the Android failure.

Comment 22

[Neil Deakin]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d4e621e02edccfff3eb4cb6b2a47655eca845280
Bug 1249522, when a file is present, only specify file type, r=smaug

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d4e621e02edc

Comment 24

[Paul Silaghi, QA [:pauly]]

Reproduced the issue on Nightly 2016-03-01.
Verified fixed FX 49.0 on Win 7, Ubuntu 14.04, OS X 10.11.