net/url_request/url_request.cc - Issue 1017583002: Set Origin header to "null" for cross origin redirects.

Unified Diff: net/url_request/url_request.cc

Issue 1017583002: Set Origin header to "null" for cross origin redirects. (Closed) Base URL: https://chromium.googlesource.com/chromium/src@master

Patch Set: Rebase on ToT Created 5 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « net/data/url_request_unittest/redirect308-to-ftp.mock-http-headers ('k') | net/url_request/url_request_unittest.cc » ('j') | net/url_request/url_request_unittest.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: net/url_request/url_request.cc

diff --git a/net/url_request/url_request.cc b/net/url_request/url_request.cc

index f84fca48064eea65ad733b1bd9ef0d89f229f2c0..23a827e6e2d5a8c5130521ffe6c00887d6813acd 100644

--- a/net/url_request/url_request.cc

+++ b/net/url_request/url_request.cc

@@ -37,6 +37,8 @@

#include "net/url_request/url_request_job_manager.h"

#include "net/url_request/url_request_netlog_params.h"

#include "net/url_request/url_request_redirect_job.h"

+#include "url/gurl.h"

+#include "url/origin.h"

using base::Time;

using std::string;

@@ -942,6 +944,29 @@ int URLRequest::Redirect(const RedirectInfo& redirect_info) {

method_ = redirect_info.new_method;

}

+ // Cross-origin redirects should result in an Origin header value of "null" if

+ // the Origin header is present in the original request (see

+ // https://tools.ietf.org/id/draft-abarth-origin-03.html#rfc.section.5). This

Ryan Sleevi 2015/03/19 03:46:34 Cite the RFC :P http://tools.ietf.org/html/rfc645

jww 2015/03/19 17:53:10 Good catch. See my comment below.

+ // is necessary to prevent a reflection of POST requests to bypass CSRF

+ // protections. If the header was not set to "null", a POST request from

Ryan Sleevi 2015/03/19 03:46:34 This doesn't align with how http://tools.ietf.org/

jww 2015/03/19 17:53:09 Well, unfortunately, my description was based on t

davidben 2015/03/24 23:47:38 I believe null is correct. Looks like we ended up

jww 2015/03/27 22:16:14 Great finds! I guess I wasn't aware of Ryan's tota

+ // origin A to a malicious origin M could be redirected by M back to A. Then

+ // if A checked the Origin header, it would appear to be a request from

+ // itself, which it might reasonably conclude would allow it to modify server

+ // state. However, this would actually be a confused deputy, since M may have

+ // carefully chosen what URL on A to redirected to, such that only A should

+ // have been able to make the request.

+ //

+ // The alternate solution is to place both origins in the Origin header, but

+ // whether to do that or set Origin to "null" is left to the discretion of the

+ // user agent.

+ //

+ // See also https://crbug.com/465517.

+ if (redirect_info.new_url.GetOrigin() != url().GetOrigin() &&

+ extra_request_headers_.HasHeader(HttpRequestHeaders::kOrigin)) {

+ extra_request_headers_.SetHeader(HttpRequestHeaders::kOrigin,

+ url::Origin().string());

+ }

referrer_ = redirect_info.new_referrer;

first_party_for_cookies_ = redirect_info.new_first_party_for_cookies;