Compile failure on Google%20Chrome%20Win

Revert "[chrome_elf, third-party block support] Pass file path in registry."

This reverts commit ea1974d4.

Reason for revert: <INSERT REASONING HERE>

Original change's description:
> [chrome_elf, third-party block support] Pass file path in registry.
> 
> Due to rare case of install_static::GetUserDataDir() path expansion,
> this API cannot be used inside DllMain().  The full, expanded path to
> the packed blacklist file will be dropped into install_static::GetRegistryPath(),
> in the kThirdPartyRegKeyName subkey, in the kBlFilePathRegValue REG_SZ value.
> This has no other impact on existing functionality.
> 
> - This change includes comments, constants, and tests.
> 
> - LogLoadAttempt() will now include the section path for any 'blocked' logs
> as well.  Friendly reminder that log.AddEntry() will still not add duplicate
> 'blocked' logs, to protect against spammy logging.
> 
> - Removed old bit of uninstaller touching old blacklist.
> 
> Test: chrome_elf_unittests.exe, ThirdParty*
> Bug: 769590
> Change-Id: Id9ada49215aa110db9fb72a4712da9d3fb7ccf31
> Reviewed-on: https://chromium-review.googlesource.com/1109600
> Reviewed-by: Robert Shield <robertshield@chromium.org>
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Commit-Queue: Penny MacNeil <pennymac@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#570849}

TBR=robertshield@chromium.org,tsepez@chromium.org,pennymac@chromium.org

Change-Id: I8f734013213e0fa1e1321e3b573172ed9a85a649
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 769590
Reviewed-on: https://chromium-review.googlesource.com/1117399Reviewed-by: Peter Mayo <petermayo@chromium.org>
Commit-Queue: Peter Mayo <petermayo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570857}

chrome/installer/setup/uninstall.cc

@@ -572,6 +572,12 @@ void RemoveBlacklistState() {
                                  install_static::GetRegistryPath().append(
                                      blacklist::kRegistryBeaconKeyName),
                                  0);  // wow64_access
+  // The following key is no longer used (https://crbug.com/631771). This
+  // cleanup is being left in for a time though.
+  InstallUtil::DeleteRegistryKey(
+      HKEY_CURRENT_USER,
+      install_static::GetRegistryPath().append(L"\\BLFinchList"),
+      0);  // wow64_access
 }
 
 // Removes the browser's persistent state in the Windows registry for the

chrome_elf/chrome_elf_main.cc

@@ -47,15 +47,6 @@ bool GetUserDataDirectoryThunk(wchar_t* user_data_dir,
   return true;
 }
 
-// DllMain
-// -------
-// Warning: The OS loader lock is held during DllMain.  Be careful.
-// https://msdn.microsoft.com/en-us/library/windows/desktop/dn633971.aspx
-//
-// - Note: Do not use install_static::GetUserDataDir from inside DllMain.
-//         This can result in path expansion that triggers secondary DLL loads,
-//         that will blow up with the loader lock held.
-//         https://bugs.chromium.org/p/chromium/issues/detail?id=748949#c18
 BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {
   if (reason == DLL_PROCESS_ATTACH) {
     install_static::InitializeProductDetailsForPrimaryModule();

chrome_elf/third_party_dlls/logs.cc

@@ -179,11 +179,13 @@ void LogLoadAttempt(LogType log_type,
            elf_sha1::kSHA1Length);
   ::memcpy(&entry.code_id_hash[0], code_id_hash.data(), elf_sha1::kSHA1Length);
 
-  // Store the full section path if the module was allowed or blocked.
-  entry.full_path = full_image_path;
-  // Edge condition.  Ensure the path length is <= max(uint32_t) - 1.
-  if (entry.full_path.size() > std::numeric_limits<uint32_t>::max() - 1)
-    entry.full_path.resize(std::numeric_limits<uint32_t>::max() - 1);
+  // Only store full path if the module was allowed to load.
+  if (log_type == LogType::kAllowed) {
+    entry.full_path = full_image_path;
+    // Edge condition.  Ensure the path length is <= max(uint32_t) - 1.
+    if (entry.full_path.size() > std::numeric_limits<uint32_t>::max() - 1)
+      entry.full_path.resize(std::numeric_limits<uint32_t>::max() - 1);
+  }
 
   // Add the new entry.
   Log& log =

chrome_elf/third_party_dlls/packed_list_file.cc

@@ -12,8 +12,7 @@
 #include <algorithm>
 #include <limits>
 
-#include "chrome/install_static/install_util.h"
-#include "chrome_elf/nt_registry/nt_registry.h"
+#include "chrome/install_static/user_data_dir.h"
 #include "chrome_elf/third_party_dlls/packed_list_format.h"
 
 namespace third_party_dlls {
@@ -27,7 +26,7 @@ bool g_initialized = false;
 PackedListModule* g_bl_module_array = nullptr;
 size_t g_bl_module_array_size = 0;
 
-// NOTE: this "global" is only initialized once on first access.
+// NOTE: this "global" is only initialized once during InitComponent().
 // NOTE: it is wrapped in a function to prevent exit-time dtors.
 std::wstring& GetBlFilePath() {
   static std::wstring* const file_path = new std::wstring();
@@ -107,38 +106,20 @@ FileStatus ReadInArray(HANDLE file,
   return FileStatus::kSuccess;
 }
 
-// Reads the path to the blacklist file from the registry into |file_path|.
-//
-// - Returns false if the value is not found, is not a REG_SZ, or is empty.
-bool GetFilePathFromRegistry(std::wstring* file_path) {
-  file_path->clear();
-  HANDLE key_handle = nullptr;
-
-  // If the ThirdParty registry key does not exist, it will be created.
-  if (!nt::CreateRegKey(nt::HKCU,
-                        install_static::GetRegistryPath()
-                            .append(kThirdPartyRegKeyName)
-                            .c_str(),
-                        KEY_QUERY_VALUE, &key_handle)) {
-    return false;
-  }
-
-  bool found = nt::QueryRegValueSZ(key_handle, kBlFilePathRegValue, file_path);
-  nt::CloseRegKey(key_handle);
-
-  return found && !file_path->empty();
-}
-
 // Open a packed data file.
 //
-// - Returns kSuccess, kFilePathNotFoundInRegistry, or kFileNotFound on success.
+// - Returns kSuccess or kFileNotFound on success.
 FileStatus OpenDataFile(HANDLE* file_handle) {
   *file_handle = INVALID_HANDLE_VALUE;
   std::wstring& file_path = GetBlFilePath();
 
   // The path may have been overridden for testing.
-  if (file_path.empty() && !GetFilePathFromRegistry(&file_path))
-    return FileStatus::kFilePathNotFoundInRegistry;
+  if (file_path.empty()) {
+    if (!install_static::GetUserDataDirectory(&file_path, nullptr))
+      return FileStatus::kUserDataDirFail;
+    file_path.append(kFileSubdir);
+    file_path.append(kBlFileName);
+  }
 
   // See if file exists.  INVALID_HANDLE_VALUE alert!
   *file_handle =
@@ -160,16 +141,17 @@ FileStatus OpenDataFile(HANDLE* file_handle) {
   return FileStatus::kSuccess;
 }
 
-// Find the packed blacklist file and read in the array.
-// - NOTE: kFilePathNotFoundInRegistry, kFileNotFound, and kArraySizeZero are
-//         treated as kSuccess.
+// Example file location, relative to user data dir.
+// %localappdata% / Google / Chrome SxS / User Data / ThirdPartyModuleList64 /
+//
+// - NOTE: kFileNotFound and kArraySizeZero are treated as kSuccess.
 FileStatus InitInternal() {
+  // blacklist
+  // ---------
   HANDLE handle = INVALID_HANDLE_VALUE;
   FileStatus status = OpenDataFile(&handle);
-  if (status == FileStatus::kFilePathNotFoundInRegistry ||
-      status == FileStatus::kFileNotFound) {
+  if (status == FileStatus::kFileNotFound)
     return FileStatus::kSuccess;
-  }
   if (status == FileStatus::kSuccess) {
     status = ReadInArray(handle, &g_bl_module_array_size, &g_bl_module_array);
     ::CloseHandle(handle);

chrome_elf/third_party_dlls/packed_list_file.h

@@ -14,7 +14,7 @@ namespace third_party_dlls {
 // "static_cast<int>(FileStatus::value)" to access underlying value.
 enum class FileStatus {
   kSuccess = 0,
-  kFilePathNotFoundInRegistry = 1,
+  kUserDataDirFail = 1,
   kFileNotFound = 2,
   kFileAccessDenied = 3,
   kFileUnexpectedFailure = 4,

chrome_elf/third_party_dlls/packed_list_file_unittest.cc

@@ -14,11 +14,8 @@
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
-#include "base/test/test_reg_util_win.h"
 #include "base/win/pe_image.h"
-#include "chrome/install_static/install_util.h"
 #include "chrome/install_static/user_data_dir.h"
-#include "chrome_elf/nt_registry/nt_registry.h"
 #include "chrome_elf/sha1/sha1.h"
 #include "chrome_elf/third_party_dlls/packed_list_format.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -29,38 +26,6 @@ namespace {
 constexpr wchar_t kTestBlFileName[] = L"blfile";
 constexpr DWORD kPageSize = 4096;
 
-void RegRedirect(nt::ROOT_KEY key,
-                 registry_util::RegistryOverrideManager* rom) {
-  ASSERT_NE(key, nt::AUTO);
-  HKEY root = (key == nt::HKCU ? HKEY_CURRENT_USER : HKEY_LOCAL_MACHINE);
-  base::string16 temp;
-
-  ASSERT_NO_FATAL_FAILURE(rom->OverrideRegistry(root, &temp));
-  ASSERT_TRUE(nt::SetTestingOverride(key, temp));
-}
-
-void CancelRegRedirect(nt::ROOT_KEY key) {
-  ASSERT_NE(key, nt::AUTO);
-  ASSERT_TRUE(nt::SetTestingOverride(key, base::string16()));
-}
-
-bool CreateRegistryKeyValue(const base::string16& full_file_path) {
-  base::win::RegKey key;
-  if (key.Create(HKEY_CURRENT_USER,
-                 install_static::GetRegistryPath()
-                     .append(kThirdPartyRegKeyName)
-                     .c_str(),
-                 KEY_WRITE) != ERROR_SUCCESS ||
-      !key.Valid()) {
-    return false;
-  }
-  if (key.WriteValue(kBlFilePathRegValue, full_file_path.c_str()) !=
-      ERROR_SUCCESS)
-    return false;
-
-  return true;
-}
-
 struct TestModule {
   std::string basename;
   DWORD timedatestamp;
@@ -254,28 +219,5 @@ TEST_F(ThirdPartyFileTest, CorruptFile) {
   EXPECT_EQ(InitFromFile(), FileStatus::kMetadataReadFail);
 }
 
-// Test successful initialization, getting the file path from registry.
-TEST_F(ThirdPartyFileTest, SuccessFromRegistry) {
-  // 1. Enable reg override for test net.
-  registry_util::RegistryOverrideManager override_manager;
-  ASSERT_NO_FATAL_FAILURE(RegRedirect(nt::HKCU, &override_manager));
-
-  // 2. Add a sample ThirdParty subkey and value, which would be created by
-  //    chrome.dll.
-  ASSERT_TRUE(CreateRegistryKeyValue(GetBlTestFilePath()));
-
-  // 3. Drop a blacklist to the expected path.
-  CreateTestFile();
-
-  // Clear override file path so that initialization goes to registry.
-  OverrideFilePathForTesting(L"");
-
-  // 4. Run the test.
-  EXPECT_EQ(InitFromFile(), FileStatus::kSuccess);
-
-  // 5. Disable reg override.
-  ASSERT_NO_FATAL_FAILURE(CancelRegRedirect(nt::HKCU));
-}
-
 }  // namespace
 }  // namespace third_party_dlls

chrome_elf/third_party_dlls/packed_list_format.cc

@@ -8,11 +8,17 @@
 
 namespace third_party_dlls {
 
-// Subkey relative to install_static::GetRegistryPath().
-const wchar_t kThirdPartyRegKeyName[] = L"\\ThirdParty";
-
-// Subkey value of type REG_SZ to hold a full path to a packed-list file.
-const wchar_t kBlFilePathRegValue[] = L"BlFilePath";
+// Subdir relative to install_static::GetUserDataDirectory().
+const wchar_t kFileSubdir[] =
+    L"\\ThirdPartyModuleList"
+#if defined(_WIN64)
+    L"64";
+#else
+    L"32";
+#endif
+
+// Packed module data cache file.
+const wchar_t kBlFileName[] = L"\\bldata";
 
 std::string GetFingerprintString(uint32_t image_size,
                                  uint32_t time_data_stamp) {

chrome_elf/third_party_dlls/packed_list_format.h

@@ -30,11 +30,11 @@ namespace third_party_dlls {
 //   second by code_id hash (there can be multiple of the same basename hash).
 // -----------------------------------------------------------------------------
 
-// Subkey relative to install_static::GetRegistryPath().
-extern const wchar_t kThirdPartyRegKeyName[];
+// Subdir relative to install_static::GetUserDataDirectory().
+extern const wchar_t kFileSubdir[];
 
-// Subkey value of type REG_SZ to hold a full path to a packed-list file.
-extern const wchar_t kBlFilePathRegValue[];
+// Packed module list cache file.
+extern const wchar_t kBlFileName[];
 
 enum PackedListVersion : uint32_t {
   kInitialVersion = 1,