Fix fuzzer crash for preg_parser

components/policy/core/common/preg_parser.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/policy/core/common/preg_parser.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 110 matching lines @@
 bool ReadFieldString(const uint8_t** cursor,
                      const uint8_t* end,
                      base::string16* str) {
   int current = -1;
   while ((current = NextChar(cursor, end)) > 0x0000)
     *str += current;
 
   return current == L'\0';
 }
 
-std::string DecodePRegStringValue(const std::vector<uint8_t>& data) {
+// Converts the UTF16 |data| to an UTF8 string |value|. Returns false if the
+// resulting UTF8 string contains invalid characters.
+bool DecodePRegStringValue(const std::vector<uint8_t>& data,
+                           std::string* value) {
   size_t len = data.size() / sizeof(base::char16);
-  if (len <= 0)
-    return std::string();
+  if (len <= 0) {
+    value->clear();
+    return true;
+  }
 
   const base::char16* chars =
       reinterpret_cast<const base::char16*>(data.data());
-  base::string16 result;
-  std::transform(chars, chars + len - 1, std::back_inserter(result),
+  base::string16 utf16_str;
+  std::transform(chars, chars + len - 1, std::back_inserter(utf16_str),
                  std::ptr_fun(base::ByteSwapToLE16));
-  return base::UTF16ToUTF8(result);
+  // Note: UTF16ToUTF8() only checks whether all chars are valid code points,
+  // but not whether they're valid characters. IsStringUTF8(), however, does.
+  *value = base::UTF16ToUTF8(utf16_str);
+  if (!base::IsStringUTF8(*value)) {
+    LOG(ERROR) << "String '" << *value << "' is not a valid UTF8 string";
+    return false;

[Thiemo Nagel, 2017/05/02 15:48:31]

Optional nit: Maybe clear |*value| as part of the error path to ensure that it
cannot be used inadvertently?

[ljusten (tachyonic), 2017/05/02 15:58:32]

Done.

+  }
+  return true;
 }
 
 // Decodes a value from a PReg file given as a uint8_t vector.
 bool DecodePRegValue(uint32_t type,
                      const std::vector<uint8_t>& data,
                      std::unique_ptr<base::Value>* value) {
+  std::string data_utf8;
   switch (type) {
     case REG_SZ:
     case REG_EXPAND_SZ:
-      value->reset(new base::Value(DecodePRegStringValue(data)));
+      if (!DecodePRegStringValue(data, &data_utf8))
+        return false;
+      value->reset(new base::Value(data_utf8));
       return true;
     case REG_DWORD_LITTLE_ENDIAN:
     case REG_DWORD_BIG_ENDIAN:
       if (data.size() == sizeof(uint32_t)) {
         uint32_t val = *reinterpret_cast<const uint32_t*>(data.data());
         if (type == REG_DWORD_BIG_ENDIAN)
           val = base::NetToHost32(val);
         else
           val = base::ByteSwapToLE32(val);
         value->reset(new base::Value(static_cast<int>(val)));
@@ skipping 62 matching lines @@
 
   std::string value_name(base::UTF16ToUTF8(value));
   if (!base::StartsWith(value_name, kActionTriggerPrefix,
                         base::CompareCase::SENSITIVE)) {
     std::unique_ptr<base::Value> value;
     if (DecodePRegValue(type, data, &value))
       dict->SetValue(value_name, std::move(value));
     return;
   }
 
+  std::string data_utf8;
   std::string action_trigger(base::ToLowerASCII(
       value_name.substr(arraysize(kActionTriggerPrefix) - 1)));
   if (action_trigger == kActionTriggerDeleteValues) {
-    for (const std::string& value :
-         base::SplitString(DecodePRegStringValue(data), ";",
-                           base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY))
-      dict->RemoveValue(value);
+    if (DecodePRegStringValue(data, &data_utf8)) {
+      for (const std::string& value :
+           base::SplitString(data_utf8, ";", base::KEEP_WHITESPACE,
+                             base::SPLIT_WANT_NONEMPTY))
+        dict->RemoveValue(value);
+    }
   } else if (base::StartsWith(action_trigger, kActionTriggerDeleteKeys,
                               base::CompareCase::SENSITIVE)) {
-    for (const std::string& key :
-         base::SplitString(DecodePRegStringValue(data), ";",
-                           base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY))
-      dict->RemoveKey(key);
+    if (DecodePRegStringValue(data, &data_utf8)) {
+      for (const std::string& key :
+           base::SplitString(data_utf8, ";", base::KEEP_WHITESPACE,
+                             base::SPLIT_WANT_NONEMPTY))
+        dict->RemoveKey(key);
+    }
   } else if (base::StartsWith(action_trigger, kActionTriggerDel,
                               base::CompareCase::SENSITIVE)) {
     dict->RemoveValue(value_name.substr(arraysize(kActionTriggerPrefix) - 1 +
                                         arraysize(kActionTriggerDel) - 1));
   } else if (base::StartsWith(action_trigger, kActionTriggerDelVals,
                               base::CompareCase::SENSITIVE)) {
     // Delete all values.
     dict->ClearValues();
   } else if (base::StartsWith(action_trigger, kActionTriggerSecureKey,
                               base::CompareCase::SENSITIVE) ||
@@ skipping 119 matching lines @@
   }
 
   LOG(ERROR) << "Error parsing PReg " << debug_name << " at offset "
              << (reinterpret_cast<const uint8_t*>(cursor - 1) - preg_data);
   *status = POLICY_LOAD_STATUS_PARSE_ERROR;
   return false;
 }
 
 }  // namespace preg_parser
 }  // namespace policy