Prevent web content from forging File entries in drag and drop.

Prevent web content from forging File entries in drag and drop.

There are two separate bugs that this and the corresponding Chrome patch
aim to address:
- On Linux, files and URLs are transferred in the same MIME type, so
  it's impossible to tell if a filename was set by a trusted source or
  forged by web content.
- DownloadURL triggers the download of potentially cross-origin content.
  On some platforms, such as Windows, the resulting download is treated
  as a file drag by Chrome, allowing web content to read cross origin
  content.

In order to prevent web content from doing this, drags initiated by a
renderer will be marked as tainted. When tainted drags are over web
content, Blink will only allow the resulting filename to be used for
navigation, with Chrome enforcing this with the sandbox policy.

Unfortunately, this does break some potentially interesting use cases
like being able to drag an attachment from Gmail to a file input, but
those will have to be separately addressed, if possible.

BUG=346135
R=abarth@chromium.org, tony@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=169711

[dcheng, 2014-03-11 00:53:21]

This is pretty ugly, needs a test, and still needs a followup patch in Chrome.
That being said, I couldn't think of a better approach. I originally wanted to
pass a FileAccessPolicy to DataObject, but that doesn't work out very well
because then you need to start filtering the returned entries in
DataObject::item()... so it's much easier to just not populate it to begin with.

kinuko@, do I need to do any work on the Chrome side to make sure we don't leak
File objects to forged files through the filesystem API?

[tony, 2014-03-11 00:58:38]

Can you add me to the bug?  This is for drag navigations?  You might have to
write a manual test for this.

[dcheng, 2014-03-11 01:02:28]

On 2014/03/11 00:58:38, tony wrote:
> Can you add me to the bug?  This is for drag navigations?  You might have to
> write a manual test for this.

I'm hoping that I can use some variant of beginDragWithFiles to test this and a
clever beforeunload handler, but we'll see how that works.

I'll also update the CL description to actually describe the approach I'm
planning to use.

[kinuko, 2014-03-11 13:09:31]

> kinuko@, do I need to do any work on the Chrome side to make sure we don't
leak
File objects to forged files through the filesystem API?

As commented on the issue for dragged filesystem I don't think we need special
care (in my belief). For DownloadURL we'll also need to take care of filesystem:
URL case (but looks like your patch will cover it)

https://codereview.chromium.org/193803002/diff/1/Source/core/page/DragData.cpp
File Source/core/page/DragData.cpp (right):

https://codereview.chromium.org/193803002/diff/1/Source/core/page/DragData.cp...
Source/core/page/DragData.cpp:73: && (m_platformDragData->containsFilenames() ||
!m_platformDragData->filenameForNavigation().isEmpty()));
nit: feels this condition is a bit hard to read... maybe could be split into
multiple if?

[tony, 2014-03-11 16:53:05]

This looks reasonable to me.  LGTM with a test, although that could be a follow
up because an automated test probably depend on the Chrome side changes.

[dcheng, 2014-03-20 23:35:31]

+abarth for OWNERS approval for public/web.

I've tested end to end on Linux with the patch currently at
https://codereview.chromium.org/207013003/ to verify that it works as
advertised. I'll need to finish up the Chrome-side patch before we can add a
layout test for this in Blink.

[abarth-chromium, 2014-03-21 00:01:42]

public/ rslgtm

[dcheng, 2014-03-21 00:03:19]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-03-21 00:03:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/193803002/1

[commit-bot: I haz the power, 2014-03-21 01:11:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-21 01:11:51]

Try jobs failed on following builders:
  tryserver.blink on linux_blink_dbg

[dcheng, 2014-03-21 01:19:42]

Committed patchset #1 manually as r169711 (presubmit successful).