Ignore suggested download filename for cross origin links.

Ignore suggested download filename for cross origin links.

The @download attribute can be used to propose a filename for a
download. If the download was initiated by a different, potentially
untrusted origin, then using the proposed filename from the @download
attribute can be a security concern. Therefore, only expose the proposed
filename after verifying that the request isn't a cross origin request.

The request can still be redirected to a different origin. Therefore the
embedder is responsible for ignoring the suggested name on a cross
origin redirect. The companion CL
https://codereview.chromium.org/246893006/ implements this for
//content.

Depends on Chromium CL: https://codereview.chromium.org/200663002/ for
test_runner changes.

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

[asanka, 2014-03-14 19:27:36]

adamk: Could you take a look?

[adamk, 2014-03-14 20:39:42]

Seems reasonable to me, but I'd be interested in looking at the bug; could you
CC me?

[asanka, 2014-03-14 20:50:51]

On 2014/03/14 20:39:42, adamk wrote:
> Seems reasonable to me, but I'd be interested in looking at the bug; could you
> CC me?

Added.

[adamk, 2014-03-14 21:37:57]

lgtm, but I'd like abarth to look at this change before it gets committed.

[asanka, 2014-03-14 22:37:11]

On 2014/03/14 21:37:57, adamk wrote:
> lgtm, but I'd like abarth to look at this change before it gets committed.

Thanks! Also +CCd abarth to the issue.

[abarth-chromium, 2014-03-17 18:17:48]

not lgtm

https://codereview.chromium.org/197033005/diff/1/Source/core/html/HTMLAnchorE...
File Source/core/html/HTMLAnchorElement.cpp (right):

https://codereview.chromium.org/197033005/diff/1/Source/core/html/HTMLAnchorE...
Source/core/html/HTMLAnchorElement.cpp:437:
frame->loader().client()->loadURLExternally(request, NavigationPolicyDownload,
suggestedName);
This check doesn't work.  The attacker can use an HTTP redirect to make the
initial URL looks same-origin but have the actual content come from an arbitrary
URL.

You probably need to do this check in the download system after you've followed
the whole redirect chain.

[asanka, 2014-03-17 18:50:24]

https://codereview.chromium.org/197033005/diff/1/Source/core/html/HTMLAnchorE...
File Source/core/html/HTMLAnchorElement.cpp (right):

https://codereview.chromium.org/197033005/diff/1/Source/core/html/HTMLAnchorE...
Source/core/html/HTMLAnchorElement.cpp:437:
frame->loader().client()->loadURLExternally(request, NavigationPolicyDownload,
suggestedName);
On 2014/03/17 18:17:48, abarth wrote:
> This check doesn't work.  The attacker can use an HTTP redirect to make the
> initial URL looks same-origin but have the actual content come from an
arbitrary
> URL.
> 
> You probably need to do this check in the download system after you've
followed
> the whole redirect chain.

Good point! Thanks.

[asanka, 2014-04-23 19:05:03]

Adam and Adam: I've revived this CL by splitting the responsibility of dealing
with cross origin redirects between Blink and //content as described in
https://codereview.chromium.org/246893006/.

There are no code changes other than merging with a more recent revision in this
CL. But I'd like abarth to check if the issues raised are addressed between this
CL and https://codereview.chromium.org/246893006/.

[abarth-chromium, 2014-04-23 20:15:31]

lgtm

[asanka, 2014-04-28 14:19:12]

The CQ bit was checked by asanka@chromium.org

[commit-bot: I haz the power, 2014-04-28 14:19:26]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asanka@chromium.org/197033005/20001

[commit-bot: I haz the power, 2014-04-28 15:28:14]

Change committed as 172767