[Backport] CVE-2023-0141: Insufficient policy enforcement in CORS (1/2)

Cherry-pick of patch originally reviewed on
https://chromium-review.googlesource.com/c/chromium/src/+/3963942:
Net: API change to take a value to evaluate safe headers

This patch changes the net::HttpUtil::IsSafeHeader() API to take
a header value in addition to the name. This API change is needed
in a coming change, and this CL is a preparation to avoid mixing
the core change with mechanical changes spread around the code base.

This also affects blink::cors::IsForbiddenHeaderName().

Bug: 1362331
Change-Id: I517799b96c3a045c336d2a509691bb8cc1f173e0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3963942
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1062009}
Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/454295
Reviewed-by: Michal Klocek <michal.klocek@qt.io>

11 files changed, 32 insertions, 30 deletions

diff --git a/chromium/chrome/browser/extensions/api/downloads/downloads_api.cc b/chromium/chrome/browser/extensions/api/downloads/downloads_api.cc
index bdbd9a08659..22a466a984d 100644
--- a/chromium/chrome/browser/extensions/api/downloads/downloads_api.cc
+++ b/chromium/chrome/browser/extensions/api/downloads/downloads_api.cc
@@ -1032,19 +1032,19 @@ ExtensionFunction::ResponseAction DownloadsDownloadFunction::Run() {
     download_params->set_prompt(*options.save_as);
 
   if (options.headers.get()) {
-    for (const downloads::HeaderNameValuePair& name_value : *options.headers) {
-      if (!net::HttpUtil::IsValidHeaderName(name_value.name)) {
+    for (const downloads::HeaderNameValuePair& header : *options.headers) {
+      if (!net::HttpUtil::IsValidHeaderName(header.name)) {
         return RespondNow(Error(download_extension_errors::kInvalidHeaderName));
       }
-      if (!net::HttpUtil::IsSafeHeader(name_value.name)) {
+      if (!net::HttpUtil::IsSafeHeader(header.name, header.value)) {
         return RespondNow(
             Error(download_extension_errors::kInvalidHeaderUnsafe));
       }
-      if (!net::HttpUtil::IsValidHeaderValue(name_value.value)) {
+      if (!net::HttpUtil::IsValidHeaderValue(header.value)) {
         return RespondNow(
             Error(download_extension_errors::kInvalidHeaderValue));
       }
-      download_params->add_request_header(name_value.name, name_value.value);
+      download_params->add_request_header(header.name, header.value);
     }
   }
 
diff --git a/chromium/net/android/android_http_util.cc b/chromium/net/android/android_http_util.cc
index 83f8831548f..0a632c4ddb2 100644
--- a/chromium/net/android/android_http_util.cc
+++ b/chromium/net/android/android_http_util.cc
@@ -20,9 +20,9 @@ jboolean JNI_HttpUtil_IsAllowedHeader(
   std::string header_name(ConvertJavaStringToUTF8(env, j_header_name));
   std::string header_value(ConvertJavaStringToUTF8(env, j_header_value));
 
-  return HttpUtil::IsValidHeaderName(header_name)
-    && HttpUtil::IsSafeHeader(header_name)
-    && HttpUtil::IsValidHeaderValue(header_value);
+  return HttpUtil::IsValidHeaderName(header_name) &&
+         HttpUtil::IsSafeHeader(header_name, header_value) &&
+         HttpUtil::IsValidHeaderValue(header_value);
 }
 
 }  // namespace net
diff --git a/chromium/net/http/http_util.cc b/chromium/net/http/http_util.cc
index 0978147da37..40ebbb4a3e9 100644
--- a/chromium/net/http/http_util.cc
+++ b/chromium/net/http/http_util.cc
@@ -326,7 +326,7 @@ bool HttpUtil::IsMethodIdempotent(base::StringPiece method) {
 }
 
 // static
-bool HttpUtil::IsSafeHeader(base::StringPiece name) {
+bool HttpUtil::IsSafeHeader(base::StringPiece name, base::StringPiece value) {
   if (base::StartsWith(name, "proxy-", base::CompareCase::INSENSITIVE_ASCII) ||
       base::StartsWith(name, "sec-", base::CompareCase::INSENSITIVE_ASCII))
     return false;
diff --git a/chromium/net/http/http_util.h b/chromium/net/http/http_util.h
index 65f64279020..86bf5c82583 100644
--- a/chromium/net/http/http_util.h
+++ b/chromium/net/http/http_util.h
@@ -96,11 +96,11 @@ class NET_EXPORT HttpUtil {
   // RFC 7231).
   static bool IsMethodIdempotent(base::StringPiece method);
 
-  // Returns true if it is safe to allow users and scripts to specify the header
-  // named |name|. Returns true for headers not in the list at
-  // https://fetch.spec.whatwg.org/#forbidden-header-name. Does not check header
-  // validity.
-  static bool IsSafeHeader(base::StringPiece name);
+  // Returns true if it is safe to allow users and scripts to specify a header
+  // with a given |name| and |value|.
+  // See https://fetch.spec.whatwg.org/#forbidden-request-header.
+  // Does not check header validity.
+  static bool IsSafeHeader(base::StringPiece name, base::StringPiece value);
 
   // Returns true if |name| is a valid HTTP header name.
   static bool IsValidHeaderName(base::StringPiece name);
diff --git a/chromium/services/network/cors/cors_util.cc b/chromium/services/network/cors/cors_util.cc
index 14fb9d81897..761c6a0de1a 100644
--- a/chromium/services/network/cors/cors_util.cc
+++ b/chromium/services/network/cors/cors_util.cc
@@ -21,7 +21,7 @@ std::vector<std::string> CorsUnsafeNotForbiddenRequestHeaderNames(
   size_t safe_list_value_size = 0;
 
   for (const auto& header : headers) {
-    if (!net::HttpUtil::IsSafeHeader(header.key))
+    if (!net::HttpUtil::IsSafeHeader(header.key, header.value))
       continue;
 
     const std::string name = base::ToLowerASCII(header.key);
diff --git a/chromium/services/network/websocket.cc b/chromium/services/network/websocket.cc
index 1b4da1a289b..06b041638c4 100644
--- a/chromium/services/network/websocket.cc
+++ b/chromium/services/network/websocket.cc
@@ -621,7 +621,7 @@ void WebSocket::AddChannel(
   for (const auto& header : additional_headers) {
     if (net::HttpUtil::IsValidHeaderName(header->name) &&
         net::HttpUtil::IsValidHeaderValue(header->value) &&
-        (net::HttpUtil::IsSafeHeader(header->name) ||
+        (net::HttpUtil::IsSafeHeader(header->name, header->value) ||
          base::EqualsCaseInsensitiveASCII(
              header->name, net::HttpRequestHeaders::kUserAgent) ||
          base::EqualsCaseInsensitiveASCII(header->name,
diff --git a/chromium/third_party/blink/renderer/core/fetch/headers.cc b/chromium/third_party/blink/renderer/core/fetch/headers.cc
index 3f4291baad2..b336fbd815e 100644
--- a/chromium/third_party/blink/renderer/core/fetch/headers.cc
+++ b/chromium/third_party/blink/renderer/core/fetch/headers.cc
@@ -107,7 +107,7 @@ void Headers::append(ScriptState* script_state,
   }
   // "4. Otherwise, if guard is |request| and |name| is a forbidden header
   //     name, return."
-  if (guard_ == kRequestGuard && cors::IsForbiddenHeaderName(name))
+  if (guard_ == kRequestGuard && cors::IsForbiddenRequestHeader(name, value))
     return;
   // 5. Otherwise, if guard is |request-no-cors|:
   if (guard_ == kRequestNoCorsGuard) {
@@ -164,9 +164,9 @@ void Headers::remove(ScriptState* script_state,
     UseCounter::Count(execution_context,
                       WebFeature::kFetchSetCookieInRequestGuardedHeaders);
   }
-  // "3. Otherwise, if guard is |request| and |name| is a forbidden header
-  //     name, return."
-  if (guard_ == kRequestGuard && cors::IsForbiddenHeaderName(name))
+  // "3. Otherwise, if guard is |request| and (|name|, '') is a forbidden
+  //     request header, return."
+  if (guard_ == kRequestGuard && cors::IsForbiddenRequestHeader(name, ""))
     return;
   // "4. Otherwise, if the context object’s guard is |request-no-cors|, |name|
   //     is not a no-CORS-safelisted request-header name, and |name| is not a
@@ -248,9 +248,9 @@ void Headers::set(ScriptState* script_state,
     UseCounter::Count(execution_context,
                       WebFeature::kFetchSetCookieInRequestGuardedHeaders);
   }
-  // "4. Otherwise, if guard is |request| and |name| is a forbidden header
-  //     name, return."
-  if (guard_ == kRequestGuard && cors::IsForbiddenHeaderName(name))
+  // "4. Otherwise, if guard is |request| and (|name|, |value|) is a forbidden
+  //     request header, return."
+  if (guard_ == kRequestGuard && cors::IsForbiddenRequestHeader(name, value))
     return;
   // "5. Otherwise, if guard is |request-no-CORS| and |name|/|value| is not a
   //     no-CORS-safelisted header, return."
diff --git a/chromium/third_party/blink/renderer/core/loader/web_associated_url_loader_impl.cc b/chromium/third_party/blink/renderer/core/loader/web_associated_url_loader_impl.cc
index 4de2d8c49cb..21f11a9a76f 100644
--- a/chromium/third_party/blink/renderer/core/loader/web_associated_url_loader_impl.cc
+++ b/chromium/third_party/blink/renderer/core/loader/web_associated_url_loader_impl.cc
@@ -88,7 +88,7 @@ class HTTPRequestHeaderValidator : public WebHTTPHeaderVisitor {
 void HTTPRequestHeaderValidator::VisitHeader(const WebString& name,
                                              const WebString& value) {
   is_safe_ = is_safe_ && IsValidHTTPToken(name) &&
-             !cors::IsForbiddenHeaderName(name) &&
+             !cors::IsForbiddenRequestHeader(name, value) &&
              IsValidHTTPHeaderValue(value);
 }
 
@@ -377,7 +377,7 @@ void WebAssociatedURLLoaderImpl::LoadAsynchronously(
       // consult it separately, if set.
       if (request.ReferrerString() !=
           blink::WebString(Referrer::ClientReferrerString())) {
-        DCHECK(cors::IsForbiddenHeaderName("Referer"));
+        DCHECK(cors::IsForbiddenRequestHeader("Referer", ""));
         // `Referer` is a forbidden header name, so we must disallow this to
         // load.
         allow_load = false;
diff --git a/chromium/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc b/chromium/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc
index 69c35483a78..32b4a71dbb5 100644
--- a/chromium/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc
+++ b/chromium/third_party/blink/renderer/core/xmlhttprequest/xml_http_request.cc
@@ -1391,9 +1391,10 @@ void XMLHttpRequest::setRequestHeader(const AtomicString& name,
     return;
   }
 
-  // "5. Terminate these steps if |name| is a forbidden header name."
+  // "5. Terminate these steps if (|name|, |value|) is a forbidden request
+  //      header."
   // No script (privileged or not) can set unsafe headers.
-  if (cors::IsForbiddenHeaderName(name)) {
+  if (cors::IsForbiddenRequestHeader(name, value)) {
     LogConsoleError(GetExecutionContext(),
                     "Refused to set unsafe header \"" + name + "\"");
     return;
diff --git a/chromium/third_party/blink/renderer/platform/loader/cors/cors.cc b/chromium/third_party/blink/renderer/platform/loader/cors/cors.cc
index 4e3c4c8017c..5f705e406c2 100644
--- a/chromium/third_party/blink/renderer/platform/loader/cors/cors.cc
+++ b/chromium/third_party/blink/renderer/platform/loader/cors/cors.cc
@@ -148,8 +148,8 @@ PLATFORM_EXPORT Vector<String> PrivilegedNoCorsHeaderNames() {
   return header_names;
 }
 
-bool IsForbiddenHeaderName(const String& name) {
-  return !net::HttpUtil::IsSafeHeader(name.Latin1());
+bool IsForbiddenRequestHeader(const String& name, const String& value) {
+  return !net::HttpUtil::IsSafeHeader(name.Latin1(), value.Latin1());
 }
 
 bool ContainsOnlyCorsSafelistedHeaders(const HTTPHeaderMap& header_map) {
diff --git a/chromium/third_party/blink/renderer/platform/loader/cors/cors.h b/chromium/third_party/blink/renderer/platform/loader/cors/cors.h
index 1f1bdbc079d..c4eb638e741 100644
--- a/chromium/third_party/blink/renderer/platform/loader/cors/cors.h
+++ b/chromium/third_party/blink/renderer/platform/loader/cors/cors.h
@@ -39,7 +39,8 @@ PLATFORM_EXPORT bool IsNoCorsSafelistedHeader(const String& name,
 PLATFORM_EXPORT bool IsPrivilegedNoCorsHeaderName(const String& name);
 PLATFORM_EXPORT bool IsNoCorsSafelistedHeaderName(const String& name);
 PLATFORM_EXPORT Vector<String> PrivilegedNoCorsHeaderNames();
-PLATFORM_EXPORT bool IsForbiddenHeaderName(const String& name);
+PLATFORM_EXPORT bool IsForbiddenRequestHeader(const String& name,
+                                              const String& value);
 PLATFORM_EXPORT bool ContainsOnlyCorsSafelistedHeaders(const HTTPHeaderMap&);
 
 PLATFORM_EXPORT bool IsOkStatus(int status);