[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/1109600Reviewed-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}

chrome/installer/setup/uninstall.cc

@@ -572,12 +572,6 @@ 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,6 +47,15 @@ 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,13 +179,11 @@ void LogLoadAttempt(LogType log_type,
            elf_sha1::kSHA1Length);
   ::memcpy(&entry.code_id_hash[0], code_id_hash.data(), elf_sha1::kSHA1Length);
 
-  // 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);
-  }
+  // 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);
 
   // Add the new entry.
   Log& log =

chrome_elf/third_party_dlls/packed_list_file.cc

@@ -12,7 +12,8 @@
 #include <algorithm>
 #include <limits>
 
-#include "chrome/install_static/user_data_dir.h"
+#include "chrome/install_static/install_util.h"
+#include "chrome_elf/nt_registry/nt_registry.h"
 #include "chrome_elf/third_party_dlls/packed_list_format.h"
 
 namespace third_party_dlls {
@@ -26,7 +27,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 during InitComponent().
+// NOTE: this "global" is only initialized once on first access.
 // NOTE: it is wrapped in a function to prevent exit-time dtors.
 std::wstring& GetBlFilePath() {
   static std::wstring* const file_path = new std::wstring();
@@ -106,20 +107,38 @@ 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 or kFileNotFound on success.
+// - Returns kSuccess, kFilePathNotFoundInRegistry, 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()) {
-    if (!install_static::GetUserDataDirectory(&file_path, nullptr))
-      return FileStatus::kUserDataDirFail;
-    file_path.append(kFileSubdir);
-    file_path.append(kBlFileName);
-  }
+  if (file_path.empty() && !GetFilePathFromRegistry(&file_path))
+    return FileStatus::kFilePathNotFoundInRegistry;
 
   // See if file exists.  INVALID_HANDLE_VALUE alert!
   *file_handle =
@@ -141,17 +160,16 @@ FileStatus OpenDataFile(HANDLE* file_handle) {
   return FileStatus::kSuccess;
 }
 
-// Example file location, relative to user data dir.
-// %localappdata% / Google / Chrome SxS / User Data / ThirdPartyModuleList64 /
-//
-// - NOTE: kFileNotFound and kArraySizeZero are treated as kSuccess.
+// Find the packed blacklist file and read in the array.
+// - NOTE: kFilePathNotFoundInRegistry, kFileNotFound, and kArraySizeZero are
+//         treated as kSuccess.
 FileStatus InitInternal() {
-  // blacklist
-  // ---------
   HANDLE handle = INVALID_HANDLE_VALUE;
   FileStatus status = OpenDataFile(&handle);
-  if (status == FileStatus::kFileNotFound)
+  if (status == FileStatus::kFilePathNotFoundInRegistry ||
+      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,
-  kUserDataDirFail = 1,
+  kFilePathNotFoundInRegistry = 1,
   kFileNotFound = 2,
   kFileAccessDenied = 3,
   kFileUnexpectedFailure = 4,

chrome_elf/third_party_dlls/packed_list_file_unittest.cc

@@ -14,8 +14,11 @@
 #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"
@@ -26,6 +29,38 @@ 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;
@@ -219,5 +254,28 @@ 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,17 +8,11 @@
 
 namespace third_party_dlls {
 
-// 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";
+// 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";
 
 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).
 // -----------------------------------------------------------------------------
 
-// Subdir relative to install_static::GetUserDataDirectory().
-extern const wchar_t kFileSubdir[];
+// Subkey relative to install_static::GetRegistryPath().
+extern const wchar_t kThirdPartyRegKeyName[];
 
-// Packed module list cache file.
-extern const wchar_t kBlFileName[];
+// Subkey value of type REG_SZ to hold a full path to a packed-list file.
+extern const wchar_t kBlFilePathRegValue[];
 
 enum PackedListVersion : uint32_t {
   kInitialVersion = 1,