Breakpad coverage for chrome_elf start up

chrome_elf/chrome_elf_util.cc

+// Copyright 2014 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/chrome_elf_util.h"
+
+#include <sddl.h>
+#include <windows.h>

[grt (UTC plus 2), 2014/02/14 16:37:33]

usually windows.h is included first. is there a reason it can't be here?

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+
+#include "base/macros.h"
+#include "base/strings/string16.h"
+
+namespace {
+
+const wchar_t kRegPathClientState[] = L"Software\\Google\\Update\\ClientState";
+const wchar_t kRegPathClientStateMedium[] =
+    L"Software\\Google\\Update\\ClientStateMedium";
+const wchar_t kRegValueUsageStats[] = L"usagestats";
+const wchar_t kUninstallArgumentsField[] = L"UninstallArguments";
+
+const wchar_t kAppGuidCanary[] =
+    L"{4ea16ac7-fd5a-47c3-875b-dbf4a2008c20}";
+const wchar_t kAppGuidGoogleChrome[] =
+    L"{8A69D345-D564-463c-AFF1-A69D9E530F96}";
+const wchar_t kAppGuidGoogleBinaries[] =
+    L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}";
+
+bool ReadKeyValueString(bool system_install, const wchar_t* key_path,
+                        const wchar_t* guid, const wchar_t* value_to_read,
+                        base::string16* value_out) {
+  HKEY h_key = NULL;
+  value_out->clear();
+
+  base::string16 full_key_path(key_path);
+  full_key_path.append(1, L'\\');
+  full_key_path.append(guid);
+
+  if (::RegOpenKeyEx(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+          full_key_path.c_str(), 0, KEY_QUERY_VALUE | KEY_WOW64_32KEY,

[grt (UTC plus 2), 2014/02/14 16:37:33]

nit: align with open paren above

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+          &h_key) != ERROR_SUCCESS) {
+    return false;
+  }
+
+  const size_t kMaxStringLength = 1024;
+  wchar_t raw_value[kMaxStringLength] = {};
+  DWORD size = sizeof(raw_value);
+  DWORD type = REG_SZ;
+  LONG result = ::RegQueryValueEx(h_key, value_to_read, 0, &type,
+      reinterpret_cast<LPBYTE>(raw_value), &size);
+
+  if (result == ERROR_SUCCESS) {
+    if (type != REG_SZ) {

[grt (UTC plus 2), 2014/02/14 16:37:33]

since an odd number of bytes doesn't make sense, how about (type != REG_SZ ||
(size & 1) != 0)

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+      result = ERROR_NOT_SUPPORTED;
+    } else if (size == 0) {
+      *raw_value = L'\0';
+    } else if (raw_value[size/sizeof(wchar_t) - 1] != L'\0') {
+      if ((size / sizeof(wchar_t)) < kMaxStringLength)
+        raw_value[size / sizeof(wchar_t)] = L'\0';
+      else
+        result = ERROR_MORE_DATA;
+    }
+  }
+
+  *value_out = raw_value;
+
+  ::RegCloseKey(h_key);
+
+  return ERROR_SUCCESS == result;
+}
+
+bool ReadKeyValueDW(bool system_install, const wchar_t* key_path,
+                    base::string16 guid, const wchar_t* value_to_read,
+                    DWORD* value_out) {
+  HKEY h_key = NULL;
+  *value_out = 0;
+
+  base::string16 full_key_path(key_path);
+  full_key_path.append(1, L'\\');
+  full_key_path.append(guid);
+
+  if (::RegOpenKeyEx(system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER,
+          full_key_path.c_str(), 0, KEY_QUERY_VALUE | KEY_WOW64_32KEY,

[grt (UTC plus 2), 2014/02/14 16:37:33]

align with open paren above

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+          &h_key) != ERROR_SUCCESS) {
+    return false;
+  }
+
+  DWORD size = sizeof(DWORD);
+  DWORD type = REG_DWORD;
+  LONG result = ::RegQueryValueEx(h_key, value_to_read, 0, &type,
+      reinterpret_cast<BYTE*>(value_out), &size);

[grt (UTC plus 2), 2014/02/14 16:37:33]

align with open paren above

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+
+  ::RegCloseKey(h_key);
+
+  return ERROR_SUCCESS == result;
+}
+
+}  // namespace
+
+bool IsCanary(const wchar_t* exe_path) {
+  return wcsstr(exe_path, L"Chrome SxS\\Application") != NULL;
+}
+
+bool IsSystemInstall(const wchar_t* exe_path) {
+  wchar_t program_dir[MAX_PATH] = {};
+  DWORD ret = ::GetEnvironmentVariable(L"PROGRAMFILES", program_dir,
+                                       arraysize(program_dir));
+  if (ret && ret < MAX_PATH && !wcsncmp(exe_path, program_dir, ret))
+    return true;
+
+  ret = ::GetEnvironmentVariable(L"PROGRAMFILES(X86)", program_dir,
+                                 arraysize(program_dir));
+  if (ret && ret < MAX_PATH && !wcsncmp(exe_path, program_dir, ret))
+    return true;
+
+  return false;
+}
+
+bool IsMultiInstall(bool is_system_install) {
+  base::string16 args;
+  if (!ReadKeyValueString(is_system_install, kRegPathClientState,
+          kAppGuidGoogleChrome, kUninstallArgumentsField, &args)) {

[grt (UTC plus 2), 2014/02/14 16:37:33]

align with open paren above

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+    return false;
+  }
+  return args.find(L"--multi-install") != base::string16::npos;
+}
+
+bool AreUsageStatsEnabled(const wchar_t* exe_path) {
+#if !defined(GOOGLE_CHROME_BUILD)
+  return false;
+#else
+  bool system_install = IsSystemInstall(exe_path);
+  base::string16 app_guid;
+
+  if (IsCanary(exe_path)) {
+    app_guid = kAppGuidCanary;
+  } else {
+    app_guid = IsMultiInstall(system_install) ? kAppGuidGoogleBinaries :
+                                                kAppGuidGoogleChrome;
+  }
+
+  DWORD out_value = 0;
+  if (system_install && ReadKeyValueDW(system_install,

[grt (UTC plus 2), 2014/02/14 16:37:33]

  if (system_install &&
      ReadKeyValueDW(system_install, kRegPathClientStateMedium, app_guid,
                     kRegValueUsageStats, &out_value)) {

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+          kRegPathClientStateMedium, app_guid, kRegValueUsageStats,
+          &out_value)) {
+    return out_value == 1;
+  }
+
+  return ReadKeyValueDW(system_install, kRegPathClientState,

[grt (UTC plus 2), 2014/02/14 16:37:33]

  return ReadKeyValueDW(system_install, kRegPathClientState, app_guid,
                        kRegValueUsageStats, &out_value) && out_value == 1;

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+      app_guid, kRegValueUsageStats, &out_value) && out_value == 1;
+#endif  // defined(GOOGLE_CHROME_BUILD)
+}
+
+bool GetUserSidString(base::string16* user_sid) {
+  // Get the current token.
+  HANDLE token = NULL;
+  if (!::OpenProcessToken(::GetCurrentProcess(), TOKEN_QUERY, &token))

[grt (UTC plus 2), 2014/02/14 16:37:33]

can you use base::win::ScopedHandle for the token? if not, you need to be sure
to CloseHandle(token) before each return following line 158. if you can't use
ScopedHandle, then you could possibly make lines 163 onward something like:

  base::string16 user_sid;
  if (::GetTokenInformation(...) &&
      user->User.Sid &&
      ::ConvertSidToStringSid(...)) {
    user_sid = sid_string;
    ::LocalFree(...);
  }
  CloseHandle(token);
  return user_sid;
}

it's probably a fine idea to make this return the string and leave it up to the
caller to check whether or not it's empty for success/failure. this is more in
line with chromium's approach to returning strings by value.

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+    return false;
+
+  DWORD size = sizeof(TOKEN_USER) + SECURITY_MAX_SID_SIZE;
+  BYTE user_bytes[sizeof(TOKEN_USER) + SECURITY_MAX_SID_SIZE] = {};

[grt (UTC plus 2), 2014/02/14 16:37:33]

  BYTE user_bytes[size] = {};

[Cait (Slow), 2014/02/14 23:12:04]

Compiler gets upset:
d:\chrome_git2\src\chrome_elf\breakpad.cc(61) : error C2057: expected constant e
xpression
d:\chrome_git2\src\chrome_elf\breakpad.cc(61) : error C2466: cannot allocate an
array of constant size 0

+  TOKEN_USER* user = reinterpret_cast<TOKEN_USER*>(&user_bytes);

[grt (UTC plus 2), 2014/02/14 16:37:33]

&user_bytes will be a pointer to the array rather than a pointer to its first
element. use either user_bytes (which will be automatically converted to a
pointer to the first element) or &user_bytes[0] (which explicitly is a pointer
to the first element). chromium style would be to use just |user_bytes|.

[Cait (Slow), 2014/02/14 23:12:04]

Done.

+
+  if (!::GetTokenInformation(token, TokenUser, user, size, &size))
+    return false;
+
+  if (!user->User.Sid)
+    return false;
+
+  // Convert the data to a string.
+  wchar_t* sid_string;
+  if (!::ConvertSidToStringSid(user->User.Sid, &sid_string))
+    return false;
+
+  *user_sid = sid_string;
+
+  ::LocalFree(sid_string);
+
+  return true;
+}