Rafael Gieschke Reporter
	Description • 8 years ago

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

	Emma Humphries ☕️🎸🧞‍♀️✨ (she/they) [:emceeaich] (Pacific Time) use needinfo
	Comment 1 • 8 years ago

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

	Neil Deakin Assignee
	Comment 2 • 8 years ago

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

	Daniel Veditz [:dveditz]
	Comment 3 • 8 years ago

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

	Kyle Huey (Exited; not receiving bugmail, old account, do not use)
	Comment 4 • 8 years ago

Is this a dupe of bug 666387?

	Neil Deakin Assignee
	Comment 5 • 8 years ago

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

	Neil Deakin Assignee
	Comment 7 • 8 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 8 • 8 years ago

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.

	Neil Deakin Assignee
	Comment 9 • 8 years ago

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.

	Jonas Sicking (:sicking) No longer reading bugmail consistently
	Comment 10 • 8 years ago

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.

	Neil Deakin Assignee
	Comment 11 • 8 years ago

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.

	Neil Deakin Assignee
	Comment 12 • 8 years ago

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

	Olli Pettay [:smaug][bugs@pettay.fi]
	Comment 13 • 8 years ago

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?

	Neil Deakin Assignee
	Comment 14 • 8 years ago

> 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

	Neil Deakin Assignee
	Comment 16 • 8 years ago

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

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 17 • 8 years ago

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

	Neil Deakin Assignee
	Comment 18 • 8 years ago

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

	Neil Deakin Assignee
	Comment 19 • 8 years ago

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

	Neil Deakin Assignee
	Comment 20 • 8 years ago

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

	Neil Deakin Assignee
	Comment 21 • 8 years ago

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

	Neil Deakin Assignee
	Comment 22 • 8 years ago

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

	Carsten Book [:Tomcat]
	Comment 23 • 8 years ago

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

	Paul Silaghi, QA [:pauly]
	Comment 24 • 8 years ago

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