Fix CORS-unsafe request-header byte

Bytes greater than 0x7f should not be considered as unsafe.

This CL also replaces "utf8" character conversions in
blink/renderer/platform/loader/cors/cors.cc to "latin1" as it's what's
done when actually converting blink::HTTPHeaderMap to
net::HttpRequestHeaders.

Bug: 824130, 902681
Change-Id: I01aacf814f1fc8a3ab8f191e1a9ec2bd01c1efee
Reviewed-on: https://chromium-review.googlesource.com/c/1335049Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608300}

services/network/public/cpp/cors/cors.cc

@@ -20,6 +20,11 @@
 #include "url/url_constants.h"
 #include "url/url_util.h"
 
+// String conversion from blink::String to std::string for header name/value
+// should be latin-1, not utf-8, as per HTTP. Note that as we use ByteString
+// as the IDL types of header name/value, a character whose code point is
+// greater than 255 has already been blocked.
+
 namespace {
 
 const char kAsterisk[] = "*";
@@ -90,10 +95,11 @@ bool IsSimilarToIntABNF(const std::string& header_value) {
 
 // https://fetch.spec.whatwg.org/#cors-unsafe-request-header-byte
 bool IsCorsUnsafeRequestHeaderByte(char c) {
-  return (c < 0x20 && c != 0x09) || c == 0x22 || c == 0x28 || c == 0x29 ||
-         c == 0x3a || c == 0x3c || c == 0x3e || c == 0x3f || c == 0x40 ||
-         c == 0x5b || c == 0x5c || c == 0x5d || c == 0x7b || c == 0x7d ||
-         c >= 0x7f;
+  const auto u = static_cast<uint8_t>(c);
+  return (u < 0x20 && u != 0x09) || u == 0x22 || u == 0x28 || u == 0x29 ||
+         u == 0x3a || u == 0x3c || u == 0x3e || u == 0x3f || u == 0x40 ||
+         u == 0x5b || u == 0x5c || u == 0x5d || u == 0x7b || u == 0x7d ||
+         u == 0x7f;
 }
 
 // |value| should be lower case.

services/network/public/cpp/cors/cors_unittest.cc

@@ -404,7 +404,7 @@ TEST_F(CORSTest, SafelistedAccept) {
   constexpr char kAllowed[] =
       "\t !#$%&'*+,-./0123456789;="
       "ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz|~";
-  for (int i = CHAR_MIN; i <= CHAR_MAX; ++i) {
+  for (int i = 0; i < 128; ++i) {
     SCOPED_TRACE(testing::Message() << "c = static_cast<char>(" << i << ")");
     char c = static_cast<char>(i);
     // 1 for the trailing null character.
@@ -414,6 +414,12 @@ TEST_F(CORSTest, SafelistedAccept) {
     EXPECT_EQ(std::find(kAllowed, end, c) != end,
               IsCORSSafelistedHeader("AccepT", std::string(1, c)));
   }
+  for (int i = 128; i <= 255; ++i) {
+    SCOPED_TRACE(testing::Message() << "c = static_cast<char>(" << i << ")");
+    char c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_TRUE(IsCORSSafelistedHeader("accept", std::string(1, c)));
+    EXPECT_TRUE(IsCORSSafelistedHeader("AccepT", std::string(1, c)));
+  }
 
   EXPECT_TRUE(IsCORSSafelistedHeader("accept", std::string(128, 'a')));
   EXPECT_FALSE(IsCORSSafelistedHeader("accept", std::string(129, 'a')));
@@ -473,7 +479,7 @@ TEST_F(CORSTest, SafelistedContentType) {
   constexpr char kAllowed[] =
       "\t !#$%&'*+,-./0123456789;="
       "ABCDEFGHIJKLMNOPQRSTUVWXYZ^_`abcdefghijklmnopqrstuvwxyz|~";
-  for (int i = CHAR_MIN; i <= CHAR_MAX; ++i) {
+  for (int i = 0; i < 128; ++i) {
     SCOPED_TRACE(testing::Message() << "c = static_cast<char>(" << i << ")");
     const char c = static_cast<char>(i);
     // 1 for the trailing null character.
@@ -484,6 +490,13 @@ TEST_F(CORSTest, SafelistedContentType) {
     EXPECT_EQ(is_allowed, IsCORSSafelistedHeader("content-type", value));
     EXPECT_EQ(is_allowed, IsCORSSafelistedHeader("cONtent-tYPe", value));
   }
+  for (int i = 128; i <= 255; ++i) {
+    SCOPED_TRACE(testing::Message() << "c = static_cast<char>(" << i << ")");
+    char c = static_cast<char>(static_cast<unsigned char>(i));
+    const std::string value = std::string("text/plain; charset=") + c;
+    EXPECT_TRUE(IsCORSSafelistedHeader("content-type", value));
+    EXPECT_TRUE(IsCORSSafelistedHeader("ConTEnt-Type", value));
+  }
 
   EXPECT_TRUE(IsCORSSafelistedHeader("content-type", "text/plain"));
   EXPECT_TRUE(IsCORSSafelistedHeader("CoNtEnt-TyPE", "text/plain"));

third_party/blink/renderer/platform/loader/cors/cors.cc

@@ -31,9 +31,7 @@ namespace {
 base::Optional<std::string> GetHeaderValue(const HTTPHeaderMap& header_map,
                                            const AtomicString& header_name) {
   if (header_map.Contains(header_name)) {
-    const AtomicString& atomic_value = header_map.Get(header_name);
-    CString string_value = atomic_value.GetString().Utf8();
-    return std::string(string_value.data(), string_value.length());
+    return WebString(header_map.Get(header_name)).Latin1();
   }
   return base::nullopt;
 }
@@ -295,15 +293,12 @@ bool CalculateCredentialsFlag(
 
 bool IsCORSSafelistedMethod(const String& method) {
   DCHECK(!method.IsNull());
-  CString utf8_method = method.Utf8();
-  return network::cors::IsCORSSafelistedMethod(
-      std::string(utf8_method.data(), utf8_method.length()));
+  return network::cors::IsCORSSafelistedMethod(WebString(method).Latin1());
 }
 
 bool IsCORSSafelistedContentType(const String& media_type) {
-  CString utf8_media_type = media_type.Utf8();
   return network::cors::IsCORSSafelistedContentType(
-      std::string(utf8_media_type.data(), utf8_media_type.length()));
+      WebString(media_type).Latin1());
 }
 
 bool IsNoCORSSafelistedHeader(const String& name, const String& value) {
@@ -327,9 +322,7 @@ Vector<String> CORSUnsafeRequestHeaderNames(const HTTPHeaderMap& headers) {
 }
 
 bool IsForbiddenHeaderName(const String& name) {
-  CString utf8_name = name.Utf8();
-  return network::cors::IsForbiddenHeader(
-      std::string(utf8_name.data(), utf8_name.length()));
+  return network::cors::IsForbiddenHeader(WebString(name).Latin1());
 }
 
 bool ContainsOnlyCORSSafelistedHeaders(const HTTPHeaderMap& header_map) {