[NtRegistry] Ensure REG_SZ and REG_MULTI_SZ are null terminated.

chrome_elf/nt_registry/nt_registry.cc

 // Copyright 2016 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 "chrome_elf/nt_registry/nt_registry.h"
 
 #include <assert.h>
 #include <stdlib.h>
 
 namespace {
@@ skipping 757 matching lines @@
 
   KEY_VALUE_FULL_INFORMATION* value_info =
       reinterpret_cast<KEY_VALUE_FULL_INFORMATION*>(new BYTE[size_needed]);
 
   // Second call to get the value.
   ntstatus = g_nt_query_value_key(key, &value_uni, KeyValueFullInformation,
                                   value_info, size_needed, &size_needed);
   if (NT_SUCCESS(ntstatus)) {
     *out_type = value_info->Type;
     *out_size = value_info->DataLength;
-    *out_buffer = new BYTE[*out_size];
-    ::memcpy(*out_buffer,
-             (reinterpret_cast<BYTE*>(value_info) + value_info->DataOffset),
-             *out_size);
+    *out_buffer = nullptr;
+    if (*out_size) {
+      *out_buffer = new BYTE[*out_size];

[robertshield, 2017/05/17 16:59:59]

totally optional, as this looks right to me, but figured I'd mention it. I
prefer to manage buffers of BYTEs using std::vector<BYTE> which minimizes the
chance of a leak by callers of this function. 

equivalent code here would be something like 
bool QueryRegKeyValue(HANDLE key, const wchar_t* value_name, ULONG* out_type,
std::vector<BYTE>* out_buffer) {
[...]

BYTE* data_offset = reinterpret_cast<BYTE*>(value_info) +
value_info->DataOffset;
out_buffer->assign(data_offset, data_offset + value_info->DataLength);
out_buffer->back() = 0;

[...]
}

}

[penny, 2017/07/19 22:25:15]

Done.  For the query function, a vector of bytes is now used.  Making the change
actually made it easier for me to insert changes in the new string terminator
helper function.  :)

+      ::memcpy(*out_buffer,
+               (reinterpret_cast<BYTE*>(value_info) + value_info->DataOffset),
+               *out_size);
+      if (*out_type == REG_SZ || *out_type == REG_MULTI_SZ)

[robertshield, 2017/05/17 16:59:59]

how about REG_EXPAND_SZ?

[penny, 2017/07/19 22:25:15]

Done. Thanks for noting this Robert.  Mine as well support EXPAND_SZ as just any
other SZ.

+        (*out_buffer)[(*out_size) - 1] = 0;
+    }
     success = true;
   }
 
   delete[] value_info;
   return success;
 }
 
 // wrapper function
 bool QueryRegValueDWORD(HANDLE key,
                         const wchar_t* value_name,
@@ skipping 297 matching lines @@
   if (!g_initialized)
     InitNativeRegApi();
 
   if (root == HKCU || (root == AUTO && !g_system_install))
     return g_HKCU_override;
 
   return g_HKLM_override;
 }
 
 };  // namespace nt