[Backport] CVE-2019-13664

[m77-attempt#2] Use navigation initiator for calculating the `Origin` header's value.

Before this CL, AddAdditionalRequestHeaders would set the Origin header
to either the destination origin (for main frame navigations) or to the
main frame's origin (for subframe navigations).  Both of these are wrong
and don't match Blink behavior (which correctly uses the initiator of
the navigation to calculate the Origin header's value).

Tbr: avi@chromium.org, nasko@chromium.org, lukasza@chromium.org, zhongyi@chromium.org
Bug: 915538
Change-Id: I3e15b218b159ec86d47d1e5d8628d32653f4b483
Cr-Original-Original-Commit-Position: refs/heads/master@{#689562}
Reviewed-by: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/branch-heads/3865@{#632}
Cr-Branched-From: 0cdcc6158160790658d1f033d3db873603250124-refs/heads/master@{#681094}
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

5 files changed, 68 insertions, 32 deletions

diff --git a/chromium/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/chromium/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index 74675948c45..92ef74abe9c 100644
--- a/chromium/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/chromium/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -6501,11 +6501,20 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, PostViaOpenUrlMsg) {
 // https://crbug.com/860807.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   GURL main_url(embedded_test_server()->GetURL(
-      "/form_that_posts_to_echoall_nocache.html"));
+      "initial-page.example.com", "/form_that_posts_to_echoall_nocache.html"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
   WebContents* web_contents = shell()->web_contents();
   EXPECT_EQ(0, web_contents->GetController().GetLastCommittedEntryIndex());
 
+  // Tweak the test page, so that it POSTs directly to the right cross-site URL
+  // (without going through the /cross-site-307/host.com handler, because it
+  // seems that such redirects do not preserve the Origin header).
+  GURL target_url(
+      embedded_test_server()->GetURL("another-site.com", "/echoall/nocache"));
+  ASSERT_TRUE(ExecJs(
+      web_contents,
+      JsReplace("document.getElementById('form').action = $1", target_url)));
+
   // Submit the form.
   TestNavigationObserver form_post_observer(web_contents, 1);
   EXPECT_TRUE(
@@ -6513,7 +6522,6 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   form_post_observer.Wait();
 
   // Verify that we arrived at the expected location.
-  GURL target_url(embedded_test_server()->GetURL("/echoall/nocache"));
   EXPECT_EQ(target_url, web_contents->GetLastCommittedURL());
   EXPECT_EQ(1, web_contents->GetController().GetLastCommittedEntryIndex());
 
@@ -6537,12 +6545,16 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
   EXPECT_EQ("text=value\n", body);
 
   // Extract the response nonce.
-  std::string old_response_nonce;
-  std::string response_nonce_extraction_script = R"(
-      domAutomationController.send(
-          document.getElementById('response-nonce').innerText); )";
-  EXPECT_TRUE(ExecuteScriptAndExtractString(
-      web_contents, response_nonce_extraction_script, &old_response_nonce));
+  std::string old_response_nonce =
+      EvalJs(web_contents,
+             "document.getElementById('response-nonce').innerText")
+          .ExtractString();
+
+  // Verify that the Origin header correctly reflects the initial initiator.
+  EXPECT_THAT(EvalJs(web_contents,
+                     "document.getElementById('request-headers').innerText")
+                  .ExtractString(),
+              ::testing::HasSubstr("Origin: http://initial-page.example.com"));
 
   // Go back.
   {
@@ -6599,10 +6611,16 @@ IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest, UncacheablePost) {
 
   // Extract the new response nonce and verify that it did change (e.g. that the
   // reload did load fresh content).
-  std::string new_response_nonce;
-  EXPECT_TRUE(ExecuteScriptAndExtractString(
-      web_contents, response_nonce_extraction_script, &new_response_nonce));
-  EXPECT_NE(new_response_nonce, old_response_nonce);
+  EXPECT_NE(old_response_nonce,
+            EvalJs(web_contents,
+                   "document.getElementById('response-nonce').innerText"));
+
+  // Verify that the Origin header correctly reflects the initial initiator.
+  // This is a regression test for https://crbug.com/915538.
+  EXPECT_THAT(EvalJs(web_contents,
+                     "document.getElementById('request-headers').innerText")
+                  .ExtractString(),
+              ::testing::HasSubstr("Origin: http://initial-page.example.com"));
 }
 
 // This test verifies that it is possible to reload a POST request that
diff --git a/chromium/content/browser/frame_host/navigation_request.cc b/chromium/content/browser/frame_host/navigation_request.cc
index 8ea7c433041..dcc6ebfe682 100644
--- a/chromium/content/browser/frame_host/navigation_request.cc
+++ b/chromium/content/browser/frame_host/navigation_request.cc
@@ -144,9 +144,13 @@ bool IsSecMetadataEnabled() {
 // This should match blink::ResourceRequest::needsHTTPOrigin.
 bool NeedsHTTPOrigin(net::HttpRequestHeaders* headers,
                      const std::string& method) {
-  // Don't add an Origin header if it is already present.
-  if (headers->HasHeader(net::HttpRequestHeaders::kOrigin))
-    return false;
+  // Blink version of this function checks if the Origin header might have
+  // already been added to |headers|.  This check is not replicated below
+  // because:
+  // 1. We want to overwrite the old (renderer-provided) header value
+  //    with a new, trustworthy (browser-provided) value.
+  // 2. The rest of the function matches the Blink version, so there should
+  //    be no discrepancies in the Origin value used.
 
   // Don't send an Origin header for GET or HEAD to avoid privacy issues.
   // For example, if an intranet page has a hyperlink to an external web
@@ -174,6 +178,7 @@ void AddAdditionalRequestHeaders(
     const std::string user_agent_override,
     bool has_user_gesture,
     base::Optional<url::Origin> initiator_origin,
+    network::mojom::ReferrerPolicy referrer_policy,
     FrameTreeNode* frame_tree_node) {
   if (!url.SchemeIsHTTPOrHTTPS())
     return;
@@ -237,23 +242,13 @@ void AddAdditionalRequestHeaders(
   }
 
   // Next, set the HTTP Origin if needed.
-  if (!NeedsHTTPOrigin(headers, method))
-    return;
-
-  // Create a unique origin.
-  url::Origin origin;
-  if (frame_tree_node->IsMainFrame()) {
-    // For main frame, the origin is the url currently loading.
-    origin = url::Origin::Create(url);
-  } else if ((frame_tree_node->active_sandbox_flags() &
-              blink::WebSandboxFlags::kOrigin) ==
-             blink::WebSandboxFlags::kNone) {
-    // The origin should be the origin of the root, except for sandboxed
-    // frames which have a unique origin.
-    origin = frame_tree_node->frame_tree()->root()->current_origin();
+  if (NeedsHTTPOrigin(headers, method)) {
+    url::Origin origin_header_value = initiator_origin.value_or(url::Origin());
+    origin_header_value = Referrer::SanitizeOriginForRequest(
+        url, origin_header_value, referrer_policy);
+    headers->SetHeader(net::HttpRequestHeaders::kOrigin,
+                       origin_header_value.Serialize());
   }
-
-  headers->SetHeader(net::HttpRequestHeaders::kOrigin, origin.Serialize());
 }
 
 // Should match the definition of
@@ -565,6 +560,7 @@ NavigationRequest::NavigationRequest(
         frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
         common_params.method, user_agent_override,
         common_params_.has_user_gesture, common_params.initiator_origin,
+        common_params_.referrer.policy,
         frame_tree_node);
 
     if (begin_params_->is_form_submission) {
diff --git a/chromium/content/public/common/referrer.cc b/chromium/content/public/common/referrer.cc
index d1128fce1dc..8ddc8a391ec 100644
--- a/chromium/content/public/common/referrer.cc
+++ b/chromium/content/public/common/referrer.cc
@@ -86,6 +86,16 @@ Referrer Referrer::SanitizeForRequest(const GURL& request,
 }
 
 // static
+url::Origin Referrer::SanitizeOriginForRequest(
+    const GURL& request,
+    const url::Origin& initiator,
+    network::mojom::ReferrerPolicy policy) {
+  Referrer fake_referrer(initiator.GetURL(), policy);
+  Referrer sanitizied_referrer = SanitizeForRequest(request, fake_referrer);
+  return url::Origin::Create(sanitizied_referrer.url);
+}
+
+// static
 void Referrer::SetReferrerForRequest(net::URLRequest* request,
                                      const Referrer& referrer) {
   request->SetReferrer(network::ComputeReferrer(referrer.url));
diff --git a/chromium/content/public/common/referrer.h b/chromium/content/public/common/referrer.h
index 35d201ffdbb..4f51ac2611d 100644
--- a/chromium/content/public/common/referrer.h
+++ b/chromium/content/public/common/referrer.h
@@ -30,6 +30,18 @@ struct CONTENT_EXPORT Referrer {
   static Referrer SanitizeForRequest(const GURL& request,
                                      const Referrer& referrer);
 
+  // Returns |initiator| origin sanitized by |policy| so that it can be used
+  // when requesting |request| URL.
+  //
+  // Note that the URL-based sanitization (e.g. SanitizeForRequest above) cannot
+  // be used for cases where the referrer URL is missing (e.g. about:blank) but
+  // the initiator origin still needs to be used (e.g. when calculating the
+  // value of the `Origin` header to use in a POST request).
+  static url::Origin SanitizeOriginForRequest(
+      const GURL& request,
+      const url::Origin& initiator,
+      network::mojom::ReferrerPolicy policy);
+
   static void SetReferrerForRequest(net::URLRequest* request,
                                     const Referrer& referrer);
 
diff --git a/chromium/net/test/embedded_test_server/default_handlers.cc b/chromium/net/test/embedded_test_server/default_handlers.cc
index 176ca7b6e0c..f0b06cb7182 100644
--- a/chromium/net/test/embedded_test_server/default_handlers.cc
+++ b/chromium/net/test/embedded_test_server/default_handlers.cc
@@ -163,7 +163,7 @@ std::unique_ptr<HttpResponse> HandleEchoAll(const HttpRequest& request) {
 
   body +=
       "</pre>"
-      "<h1>Request Headers:</h1><pre>" +
+      "<h1>Request Headers:</h1><pre id='request-headers'>" +
       request.all_headers + "</pre>" +
       "<h1>Response nonce:</h1><pre id='response-nonce'>" +
       base::UnguessableToken::Create().ToString() + "</pre></body></html>";