Changed UTF8 encoding check to include '\0' bytes

Changed UTF8 encoding check function in CBOR decoder to include
bytes after first embedded NULL byte.

Failed cluster fuzz test case : \63\00\00\A6

Bug: 793587
Change-Id: If519678d29568451c121617a2b02079345a14307
Reviewed-on: https://chromium-review.googlesource.com/818731
Commit-Queue: Jun Choi <hongjunchoi@chromium.org>
Reviewed-by: Chris Palmer <palmer@chromium.org>
Reviewed-by: Balazs Engedy <engedy@chromium.org>
Reviewed-by: Jeffrey Yasskin <jyasskin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524021}

content/browser/webauth/cbor/cbor_reader.cc

@@ -231,7 +231,7 @@ bool CBORReader::CheckDuplicateKey(const std::string& new_key,
 }
 
 bool CBORReader::HasValidUTF8Format(const std::string& string_data) {
-  if (!base::IsStringUTF8(string_data.data())) {
+  if (!base::IsStringUTF8(string_data)) {
     error_code_ = DecoderError::INVALID_UTF8;
     return false;
   }

content/browser/webauth/cbor/cbor_reader_unittest.cc

@@ -3,7 +3,6 @@
 // found in the LICENSE file.
 
 #include "content/browser/webauth/cbor/cbor_reader.h"
-#include "base/strings/stringprintf.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -112,8 +111,7 @@ TEST(CBORReaderTest, TestReadString) {
 
   for (const StringTestCase& test_case : kStringTestCases) {
     testing::Message scope_message;
-    scope_message << "testing string value : "
-                  << base::StringPrintf("%s", test_case.value.data());
+    scope_message << "testing string value : " << test_case.value;
     SCOPED_TRACE(scope_message);
 
     base::Optional<CBORValue> cbor = CBORReader::Read(test_case.cbor_data);
@@ -123,6 +121,50 @@ TEST(CBORReaderTest, TestReadString) {
   }
 }
 
+TEST(CBORReaderTest, TestReadStringWithNUL) {
+  static const struct {
+    const std::string value;
+    const std::vector<uint8_t> cbor_data;
+  } kStringTestCases[] = {
+      {std::string("string_without_nul"),
+       {0x72, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x5F, 0x77, 0x69, 0x74, 0x68,
+        0x6F, 0x75, 0x74, 0x5F, 0x6E, 0x75, 0x6C}},
+      {std::string("nul_terminated_string\0", 22),
+       {0x76, 0x6E, 0x75, 0x6C, 0x5F, 0x74, 0x65, 0x72, 0x6D, 0x69, 0x6E, 0x61,
+        0x74, 0x65, 0x64, 0x5F, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x00}},
+      {std::string("embedded\0nul", 12),
+       {0x6C, 0x65, 0x6D, 0x62, 0x65, 0x64, 0x64, 0x65, 0x64, 0x00, 0x6E, 0x75,
+        0x6C}},
+      {std::string("trailing_nuls\0\0", 15),
+       {0x6F, 0x74, 0x72, 0x61, 0x69, 0x6C, 0x69, 0x6E, 0x67, 0x5F, 0x6E, 0x75,
+        0x6C, 0x73, 0x00, 0x00}},
+  };
+
+  for (const auto& test_case : kStringTestCases) {
+    SCOPED_TRACE(testing::Message()
+                 << "testing string with nul bytes :" << test_case.value);
+
+    base::Optional<CBORValue> cbor = CBORReader::Read(test_case.cbor_data);
+    ASSERT_TRUE(cbor.has_value());
+    ASSERT_EQ(cbor.value().type(), CBORValue::Type::STRING);
+    EXPECT_EQ(cbor.value().GetString(), test_case.value);
+  }
+}
+
+TEST(CBORReaderTest, TestReadStringWithInvalidByteSequenceAfterNUL) {
+  // UTF-8 validation should not stop at the first NUL character in the string.
+  // That is, a string with an invalid byte sequence should fail UTF-8
+  // validation even if the invalid character is located after one or more NUL
+  // characters. Here, 0xA6 is an unexpected continuation byte.
+  static const std::vector<uint8_t> string_with_invalid_continuation_byte = {
+      0x63, 0x00, 0x00, 0xA6};
+  CBORReader::DecoderError error_code;
+  base::Optional<CBORValue> cbor =
+      CBORReader::Read(string_with_invalid_continuation_byte, &error_code);
+  EXPECT_FALSE(cbor.has_value());
+  EXPECT_EQ(error_code, CBORReader::DecoderError::INVALID_UTF8);
+}
+
 TEST(CBORReaderTest, TestReadArray) {
   static const std::vector<uint8_t> kArrayTestCaseCbor = {
       // clang-format off

content/browser/webauth/cbor/cbor_values.h

@@ -109,6 +109,7 @@ class CONTENT_EXPORT CBORValue {
   // These will all fatally assert if the type doesn't match.
   uint64_t GetUnsigned() const;
   const BinaryValue& GetBytestring() const;
+  // Returned string may contain NUL characters.
   const std::string& GetString() const;
   const ArrayValue& GetArray() const;
   const MapValue& GetMap() const;