[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}diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index f8bbe38..b80e2f6b 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -572,12 +572,6 @@
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
diff --git a/chrome_elf/chrome_elf_main.cc b/chrome_elf/chrome_elf_main.cc
index 21242927..0bc157f 100644
--- a/chrome_elf/chrome_elf_main.cc
+++ b/chrome_elf/chrome_elf_main.cc
@@ -47,6 +47,15 @@
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();
diff --git a/chrome_elf/third_party_dlls/logs.cc b/chrome_elf/third_party_dlls/logs.cc
index 48815e0..d827e61 100644
--- a/chrome_elf/third_party_dlls/logs.cc
+++ b/chrome_elf/third_party_dlls/logs.cc
@@ -179,13 +179,11 @@
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 =
diff --git a/chrome_elf/third_party_dlls/packed_list_file.cc b/chrome_elf/third_party_dlls/packed_list_file.cc
index 86aadcb37..17a71f7c 100644
--- a/chrome_elf/third_party_dlls/packed_list_file.cc
+++ b/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 @@
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 @@
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 @@
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);
diff --git a/chrome_elf/third_party_dlls/packed_list_file.h b/chrome_elf/third_party_dlls/packed_list_file.h
index 9dab05ad..6eebc057 100644
--- a/chrome_elf/third_party_dlls/packed_list_file.h
+++ b/chrome_elf/third_party_dlls/packed_list_file.h
@@ -14,7 +14,7 @@
// "static_cast<int>(FileStatus::value)" to access underlying value.
enum class FileStatus {
kSuccess = 0,
- kUserDataDirFail = 1,
+ kFilePathNotFoundInRegistry = 1,
kFileNotFound = 2,
kFileAccessDenied = 3,
kFileUnexpectedFailure = 4,
diff --git a/chrome_elf/third_party_dlls/packed_list_file_unittest.cc b/chrome_elf/third_party_dlls/packed_list_file_unittest.cc
index c7a7b13..9410ff4 100644
--- a/chrome_elf/third_party_dlls/packed_list_file_unittest.cc
+++ b/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 @@
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 @@
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
diff --git a/chrome_elf/third_party_dlls/packed_list_format.cc b/chrome_elf/third_party_dlls/packed_list_format.cc
index d93898c..c2f8d1c 100644
--- a/chrome_elf/third_party_dlls/packed_list_format.cc
+++ b/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
+// Subkey relative to install_static::GetRegistryPath().
+const wchar_t kThirdPartyRegKeyName[] = L"\\ThirdParty";
-// Packed module data cache file.
-const wchar_t kBlFileName[] = L"\\bldata";
+// 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) {
diff --git a/chrome_elf/third_party_dlls/packed_list_format.h b/chrome_elf/third_party_dlls/packed_list_format.h
index 719a6697..7ede12a 100644
--- a/chrome_elf/third_party_dlls/packed_list_format.h
+++ b/chrome_elf/third_party_dlls/packed_list_format.h
@@ -30,11 +30,11 @@
// 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,
