Add fuzzer test 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>
 #include <functional>
 #include <iterator>
 #include <limits>
 #include <memory>
 #include <string>
 #include <utility>
 #include <vector>
 
 #include "base/files/file_path.h"
 #include "base/files/memory_mapped_file.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/sys_byteorder.h"
 #include "base/values.h"
-#include "components/policy/core/common/policy_load_status.h"
 #include "components/policy/core/common/registry_dict.h"
 
 #if defined(OS_WIN)
 #include "windows.h"
 #else
 // Registry data type constants.
 #define REG_NONE 0
 #define REG_SZ 1
 #define REG_EXPAND_SZ 2
 #define REG_BINARY 3
@@ skipping 26 matching lines @@
 
 // Magic strings for the PReg value field to trigger special actions.
 const char kActionTriggerPrefix[] = "**";
 const char kActionTriggerDeleteValues[] = "deletevalues";
 const char kActionTriggerDel[] = "del.";
 const char kActionTriggerDelVals[] = "delvals";
 const char kActionTriggerDeleteKeys[] = "deletekeys";
 const char kActionTriggerSecureKey[] = "securekey";
 const char kActionTriggerSoft[] = "soft";
 
+// Same as base::EqualsCaseInsensitiveASCII, but with base::string16 args.
+bool EqualsCaseInsensitiveASCII(const base::string16& a,
+                                const base::string16& b) {
+  return base::EqualsCaseInsensitiveASCII(a, b);
+}
+
 // Returns the character at |cursor| and increments it, unless the end is here
 // in which case -1 is returned. The calling code must guarantee that
 // end - *cursor does not overflow ptrdiff_t.
 int NextChar(const uint8_t** cursor, const uint8_t* end) {
   // Only read the character if a full base::char16 is available.
   // This comparison makes sure no overflow can happen.
   if (*cursor >= end ||
       end - *cursor < static_cast<ptrdiff_t>(sizeof(base::char16)))
     return -1;
 
@@ skipping 84 matching lines @@
     case REG_FULL_RESOURCE_DESCRIPTOR:
     case REG_RESOURCE_REQUIREMENTS_LIST:
     case REG_QWORD_LITTLE_ENDIAN:
     default:
       LOG(ERROR) << "Unsupported registry data type " << type;
   }
 
   return false;
 }
 
+// Splits a registry key A\B\C into parts {A, B, C}.
+std::vector<base::string16> SplitKey(const base::string16& key_name) {
+  return base::SplitString(key_name, kRegistryPathSeparator,
+                           base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
+}
+
 // Adds |value| and |data| to |dict| or an appropriate sub-dictionary indicated
-// by |key_name|. Creates sub-dictionaries if necessary. Also handles special
+// by |key_parts|. Creates sub-dictionaries if necessary. Also handles special
 // action triggers, see |kActionTrigger*|, that can, for instance, remove an
 // existing value.
-void HandleRecord(const base::string16& key_name,
+void HandleRecord(const std::vector<base::string16>& key_parts,
                   const base::string16& value,
                   uint32_t type,
                   const std::vector<uint8_t>& data,
                   RegistryDict* dict) {
   // Locate/create the dictionary to place the value in.
   std::vector<base::string16> path;
 
-  for (const base::string16& entry :
-       base::SplitString(key_name, kRegistryPathSeparator,
-                         base::KEEP_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) {
+  for (const base::string16& entry : key_parts) {
     if (entry.empty())
       continue;
     const std::string name = base::UTF16ToUTF8(entry);
     RegistryDict* subdict = dict->GetKey(name);
     if (!subdict) {
       subdict = new RegistryDict();
       dict->SetKey(name, base::WrapUnique(subdict));
     }
     dict = subdict;
   }
 
   if (value.empty())
     return;
 
   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 action_trigger(base::ToLowerASCII(value_name.substr(
-      arraysize(kActionTriggerPrefix) - 1)));
+  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);
   } 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);
   } else if (base::StartsWith(action_trigger, kActionTriggerDel,
                               base::CompareCase::SENSITIVE)) {
-  dict->RemoveValue(
-        value_name.substr(arraysize(kActionTriggerPrefix) - 1 +
-                          arraysize(kActionTriggerDel) - 1));
+    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) ||
              base::StartsWith(action_trigger, kActionTriggerSoft,
                               base::CompareCase::SENSITIVE)) {
     // Doesn't affect values.
   } else {
     LOG(ERROR) << "Bad action trigger " << value_name;
   }
 }
 
 }  // namespace
 
 namespace policy {
 namespace preg_parser {
 
-const char kPRegFileHeader[8] =
-    { 'P', 'R', 'e', 'g', '\x01', '\x00', '\x00', '\x00' };
+const char kPRegFileHeader[8] = {'P',    'R',    'e',    'g',
+                                 '\x01', '\x00', '\x00', '\x00'};
 
 bool ReadFile(const base::FilePath& file_path,
               const base::string16& root,
               RegistryDict* dict,
-              PolicyLoadStatusSample* status) {
+              PolicyLoadStatusSample* status_sample) {
   base::MemoryMappedFile mapped_file;
   if (!mapped_file.Initialize(file_path) || !mapped_file.IsValid()) {
     PLOG(ERROR) << "Failed to map " << file_path.value();
-    status->Add(POLICY_LOAD_STATUS_READ_ERROR);
+    status_sample->Add(POLICY_LOAD_STATUS_READ_ERROR);
     return false;
   }
 
-  if (mapped_file.length() > kMaxPRegFileSize) {
-    LOG(ERROR) << "PReg file " << file_path.value() << " too large: "
-               << mapped_file.length();
-    status->Add(POLICY_LOAD_STATUS_TOO_BIG);
+  PolicyLoadStatus status = POLICY_LOAD_STATUS_SIZE;
+  bool res =
+      ReadData(mapped_file.data(), mapped_file.length(), root, dict, &status,
+               base::StringPrintf("file '%s'", file_path.value().c_str()));
+  if (!res) {
+    DCHECK(status != POLICY_LOAD_STATUS_SIZE);
+    status_sample->Add(status);
+  }
+  return res;
+}
+
+POLICY_EXPORT bool ReadData(const uint8_t* data,
+                            size_t data_size,
+                            const base::string16& root,
+                            RegistryDict* dict,
+                            PolicyLoadStatus* status,
+                            const std::string& debug_name) {
+  DCHECK(status);
+
+  // Check data size.
+  if (data_size > kMaxPRegFileSize) {
+    LOG(ERROR) << "PReg " << debug_name << " too large: " << data_size;
+    *status = POLICY_LOAD_STATUS_TOO_BIG;
     return false;
   }
 
   // Check the header.
   const int kHeaderSize = arraysize(kPRegFileHeader);
-  if (mapped_file.length() < kHeaderSize ||
-      memcmp(kPRegFileHeader, mapped_file.data(), kHeaderSize) != 0) {
-    LOG(ERROR) << "Bad policy file " << file_path.value();
-    status->Add(POLICY_LOAD_STATUS_PARSE_ERROR);
+  if (!data || data_size < kHeaderSize ||
+      memcmp(kPRegFileHeader, data, kHeaderSize) != 0) {
+    LOG(ERROR) << "Bad PReg " << debug_name;
+    *status = POLICY_LOAD_STATUS_PARSE_ERROR;
     return false;
   }
 
-  // Parse file contents, which is UCS-2 and little-endian. The latter I
+  // Split |root| into parts. It is important that we don't do a simple
+  // StartsWith check for |root| in the |key_name| (see below) since then root =
+  // "someroot" would trigger in key_name = "somerooting\data".

[Mattias Nissler (ping if slow), 2017/04/07 08:13:26]

Alternatively you could enforce that the prefix we test against ends with a
backslash, or that the matching prefix is followed by a backslash, both of which
seem simpler?

[ljusten (tachyonic), 2017/04/10 10:22:51]

I thought about that, I was a bit scared by the number of cases (including root
having a backslash in the end or not), but it's probably still simpler. I'll
give it a shot.

+  const std::vector<base::string16> root_parts = SplitKey(root);
+
+  // Parse data, which is expected to be UCS-2 and little-endian. The latter I
   // couldn't find documentation on, but the example I saw were all
   // little-endian. It'd be interesting to check on big-endian hardware.
-  const uint8_t* cursor = mapped_file.data() + kHeaderSize;
-  const uint8_t* end = mapped_file.data() + mapped_file.length();
+  const uint8_t* cursor = data + kHeaderSize;
+  const uint8_t* end = data + data_size;
   while (true) {
     if (cursor == end)
       return true;
 
     if (NextChar(&cursor, end) != kDelimBracketOpen)
       break;
 
     // Read the record fields.
     base::string16 key_name;
     base::string16 value;
@@ skipping 29 matching lines @@
       data.resize(size);
       if (!ReadFieldBinary(&cursor, end, size, data.data()))
         break;
       current = NextChar(&cursor, end);
     }
 
     if (current != kDelimBracketClose)
       break;
 
     // Process the record if it is within the |root| subtree.
-    if (base::StartsWith(key_name, root, base::CompareCase::INSENSITIVE_ASCII))
-      HandleRecord(key_name.substr(root.size()), value, type, data, dict);
+    std::vector<base::string16> key_parts = SplitKey(key_name);
+    if (key_parts.size() >= root_parts.size() &&
+        std::equal(root_parts.begin(), root_parts.end(), key_parts.begin(),
+                   &EqualsCaseInsensitiveASCII)) {
+      key_parts.erase(key_parts.begin(), key_parts.begin() + root_parts.size());
+      HandleRecord(key_parts, value, type, data, dict);
+    }
   }
 
-  LOG(ERROR) << "Error parsing " << file_path.value() << " at offset "
-             << reinterpret_cast<const uint8_t*>(cursor - 1) -
-                    mapped_file.data();
-  status->Add(POLICY_LOAD_STATUS_PARSE_ERROR);
+  LOG(ERROR) << "Error parsing PReg " << debug_name << " at offset "
+             << (reinterpret_cast<const uint8_t*>(cursor - 1) - data);
+  *status = POLICY_LOAD_STATUS_PARSE_ERROR;
   return false;
 }
 
 }  // namespace preg_parser
 }  // namespace policy