Remove unsafe registry APIs from base::win::RegKey

base/win/registry.cc

 // Copyright (c) 2012 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 "base/win/registry.h"
 
 #include <shlwapi.h>
 #include <stddef.h>
 #include <algorithm>
 
@@ skipping 16 matching lines @@
 // Registry values are read as BYTE* but can have wchar_t* data whose last
 // wchar_t is truncated. This function converts the reported |byte_size| to
 // a size in wchar_t that can store a truncated wchar_t if necessary.
 inline DWORD to_wchar_size(DWORD byte_size) {
   return (byte_size + sizeof(wchar_t) - 1) / sizeof(wchar_t);
 }
 
 // Mask to pull WOW64 access flags out of REGSAM access.
 const REGSAM kWow64AccessMask = KEY_WOW64_32KEY | KEY_WOW64_64KEY;
 
+#if DCHECK_IS_ON()
+inline bool isValidRootKey(HKEY key) {
+  return key == HKEY_CLASSES_ROOT || key == HKEY_CURRENT_USER ||
+         key == HKEY_LOCAL_MACHINE || key == HKEY_USERS ||
+         key == HKEY_PERFORMANCE_DATA || key == HKEY_PERFORMANCE_TEXT ||
+         key == HKEY_PERFORMANCE_NLSTEXT || key == HKEY_CURRENT_CONFIG ||
+         key == HKEY_DYN_DATA || key == HKEY_CURRENT_USER_LOCAL_SETTINGS;
+}
+#endif  // DCHECK_IS_ON()
+
 }  // namespace
 
 // Watches for modifications to a key.
 class RegKey::Watcher : public ObjectWatcher::Delegate {
  public:
   Watcher() {}
   ~Watcher() override {}
 
+  Watcher(Watcher&&) = default;
+  Watcher& operator=(Watcher&&) = default;
+
   bool StartWatching(HKEY key, const ChangeCallback& callback);
 
   // Implementation of ObjectWatcher::Delegate.
   void OnObjectSignaled(HANDLE object) override {
     DCHECK(watch_event_.IsValid() && watch_event_.Get() == object);
     ChangeCallback callback = callback_;
     callback_.Reset();
     callback.Run();
   }
 
@@ skipping 29 matching lines @@
 
   callback_ = callback;
   return object_watcher_.StartWatchingOnce(watch_event_.Get(), this);
 }
 
 // RegKey ----------------------------------------------------------------------
 
 RegKey::RegKey() : key_(NULL), wow64access_(0) {
 }
 
-RegKey::RegKey(HKEY key) : key_(key), wow64access_(0) {
+RegKey::RegKey(HKEY rootkey) : key_(rootkey), wow64access_(0) {
+  DCHECK(isValidRootKey(rootkey));
 }
 
 RegKey::RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access)
     : key_(NULL),
       wow64access_(0) {
   if (rootkey) {
     if (access & (KEY_SET_VALUE | KEY_CREATE_SUB_KEY | KEY_CREATE_LINK))
       Create(rootkey, subkey, access);
     else
       Open(rootkey, subkey, access);
   } else {
     DCHECK(!subkey);
     wow64access_ = access & kWow64AccessMask;
   }
 }
 
 RegKey::~RegKey() {
   Close();
 }
 
+RegKey::RegKey(RegKey&&) = default;
+
+RegKey& RegKey::operator=(RegKey&&) = default;
+
 LONG RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
+  DCHECK(isValidRootKey(rootkey));
   DWORD disposition_value;
   return CreateWithDisposition(rootkey, subkey, &disposition_value, access);
 }
 
 LONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
                                    DWORD* disposition, REGSAM access) {
   DCHECK(rootkey && subkey && access && disposition);
+  DCHECK(isValidRootKey(rootkey));
   HKEY subhkey = NULL;
   LONG result = RegCreateKeyEx(rootkey, subkey, 0, NULL,
                                REG_OPTION_NON_VOLATILE, access, NULL, &subhkey,
                                disposition);
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
     wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
 }
 
 LONG RegKey::CreateKey(const wchar_t* name, REGSAM access) {
   DCHECK(name && access);
+  DCHECK(key_);

[grt (UTC plus 2), 2017/06/29 08:49:47]

i'm not sure about this. when you don't care about error handling, it's nice to
be able to open a key and try to operate on it. if the open fails, so does
everything else. does RegCreateKeyEx return a reasonable result if key_ is 0?

   // After the application has accessed an alternate registry view using one of
   // the [KEY_WOW64_32KEY / KEY_WOW64_64KEY] flags, all subsequent operations
   // (create, delete, or open) on child registry keys must explicitly use the
   // same flag. Otherwise, there can be unexpected behavior.
   // http://msdn.microsoft.com/en-us/library/windows/desktop/aa384129.aspx.
   if ((access & kWow64AccessMask) != wow64access_) {
     NOTREACHED();
     return ERROR_INVALID_PARAMETER;
   }
   HKEY subkey = NULL;
   LONG result = RegCreateKeyEx(key_, name, 0, NULL, REG_OPTION_NON_VOLATILE,
                                access, NULL, &subkey, NULL);
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subkey;
     wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
 }
 
 LONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   DCHECK(rootkey && subkey && access);
+  DCHECK(isValidRootKey(rootkey));
   HKEY subhkey = NULL;
 
   LONG result = RegOpenKeyEx(rootkey, subkey, 0, access, &subhkey);
   if (result == ERROR_SUCCESS) {
     Close();
     key_ = subhkey;
     wow64access_ = access & kWow64AccessMask;
   }
 
   return result;
 }
 
 LONG RegKey::OpenKey(const wchar_t* relative_key_name, REGSAM access) {
   DCHECK(relative_key_name && access);
+  DCHECK(key_);
   // After the application has accessed an alternate registry view using one of
   // the [KEY_WOW64_32KEY / KEY_WOW64_64KEY] flags, all subsequent operations
   // (create, delete, or open) on child registry keys must explicitly use the
   // same flag. Otherwise, there can be unexpected behavior.
   // http://msdn.microsoft.com/en-us/library/windows/desktop/aa384129.aspx.
   if ((access & kWow64AccessMask) != wow64access_) {
     NOTREACHED();
     return ERROR_INVALID_PARAMETER;
   }
   HKEY subkey = NULL;
@@ skipping 10 matching lines @@
 }
 
 void RegKey::Close() {
   if (key_) {
     ::RegCloseKey(key_);
     key_ = NULL;
     wow64access_ = 0;
   }
 }
 
-// TODO(wfh): Remove this and other unsafe methods. See http://crbug.com/375400
-void RegKey::Set(HKEY key) {
-  if (key_ != key) {
-    Close();
-    key_ = key;
-  }
-}
-
-HKEY RegKey::Take() {
-  DCHECK_EQ(wow64access_, 0u);
-  HKEY key = key_;
-  key_ = NULL;
-  return key;
-}
-
 bool RegKey::HasValue(const wchar_t* name) const {
+  DCHECK(key_);
   return RegQueryValueEx(key_, name, 0, NULL, NULL, NULL) == ERROR_SUCCESS;
 }
 
 DWORD RegKey::GetValueCount() const {
+  DCHECK(key_);
   DWORD count = 0;
   LONG result = RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL, NULL, &count,
                                 NULL, NULL, NULL, NULL);
   return (result == ERROR_SUCCESS) ? count : 0;
 }
 
 LONG RegKey::GetValueNameAt(int index, std::wstring* name) const {
+  DCHECK(key_);
   wchar_t buf[256];
   DWORD bufsize = arraysize(buf);
   LONG r = ::RegEnumValue(key_, index, buf, &bufsize, NULL, NULL, NULL, NULL);
   if (r == ERROR_SUCCESS)
     *name = buf;
 
   return r;
 }
 
 LONG RegKey::DeleteKey(const wchar_t* name) {
@@ skipping 104 matching lines @@
     }
   }
 
   return result;
 }
 
 LONG RegKey::ReadValue(const wchar_t* name,
                        void* data,
                        DWORD* dsize,
                        DWORD* dtype) const {
+  DCHECK(key_);
   LONG result = RegQueryValueEx(key_, name, 0, dtype,
                                 reinterpret_cast<LPBYTE>(data), dsize);
   return result;
 }
 
 LONG RegKey::ReadValues(const wchar_t* name,
                         std::vector<std::wstring>* values) {
   values->clear();
 
   DWORD type = REG_MULTI_SZ;
@@ skipping 31 matching lines @@
 LONG RegKey::WriteValue(const wchar_t * name, const wchar_t* in_value) {
   return WriteValue(name, in_value,
       static_cast<DWORD>(sizeof(*in_value) * (wcslen(in_value) + 1)), REG_SZ);
 }
 
 LONG RegKey::WriteValue(const wchar_t* name,
                         const void* data,
                         DWORD dsize,
                         DWORD dtype) {
   DCHECK(data || !dsize);
+  DCHECK(key_);
 
   LONG result = RegSetValueEx(key_, name, 0, dtype,
       reinterpret_cast<LPBYTE>(const_cast<void*>(data)), dsize);
   return result;
 }
 
 bool RegKey::StartWatching(const ChangeCallback& callback) {
   if (!key_watcher_)
     key_watcher_.reset(new Watcher());
 
@@ skipping 197 matching lines @@
                                          REGSAM wow64access) {
   Initialize(root_key, folder_key, wow64access);
 }
 
 RegistryKeyIterator::~RegistryKeyIterator() {
   if (key_)
     ::RegCloseKey(key_);
 }
 
 DWORD RegistryKeyIterator::SubkeyCount() const {
+  DCHECK(key_);
   DWORD count = 0;
   LONG result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
                                   NULL, NULL, NULL, NULL, NULL);
   if (result != ERROR_SUCCESS)
     return 0;
 
   return count;
 }
 
 bool RegistryKeyIterator::Valid() const {
@@ skipping 38 matching lines @@
     } else {
       index_ = count - 1;
     }
   }
 
   Read();
 }
 
 }  // namespace win
 }  // namespace base