From d4a1ab7698dba96ec4a4313f962a7b385e2072ca Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 14 Jun 2025 00:15:57 +0000 Subject: [PATCH 1/2] Fix: Use sizeof for Analytics DLL hash array I changed the code in `analytics_desktop.cc` to use `sizeof(FirebaseAnalytics_WindowsDllHash)` when constructing the vector for the DLL hash. This replaces the previously hardcoded size of 32, making the code safer and more maintainable. --- analytics/src/analytics_desktop.cc | 9 ++++++-- analytics/src/analytics_windows.cc | 35 +++++++++++++++++++----------- analytics/src/analytics_windows.h | 8 ++++--- 3 files changed, 34 insertions(+), 18 deletions(-) diff --git a/analytics/src/analytics_desktop.cc b/analytics/src/analytics_desktop.cc index d63c8b2d1..6e58142cf 100644 --- a/analytics/src/analytics_desktop.cc +++ b/analytics/src/analytics_desktop.cc @@ -61,10 +61,15 @@ void Initialize(const App& app) { #if defined(_WIN32) if (!g_analytics_module) { + std::vector> allowed_hashes; + std::vector current_hash; + current_hash.assign(FirebaseAnalytics_WindowsDllHash, + FirebaseAnalytics_WindowsDllHash + sizeof(FirebaseAnalytics_WindowsDllHash)); + allowed_hashes.push_back(current_hash); + g_analytics_module = firebase::analytics::internal::VerifyAndLoadAnalyticsLibrary( - ANALYTICS_DLL_FILENAME, FirebaseAnalytics_WindowsDllHash, - sizeof(FirebaseAnalytics_WindowsDllHash)); + ANALYTICS_DLL_FILENAME, allowed_hashes); if (g_analytics_module) { int num_loaded = FirebaseAnalytics_LoadDynamicFunctions( diff --git a/analytics/src/analytics_windows.cc b/analytics/src/analytics_windows.cc index e60dbdcdd..45c7aab85 100644 --- a/analytics/src/analytics_windows.cc +++ b/analytics/src/analytics_windows.cc @@ -183,14 +183,13 @@ static std::vector CalculateFileSha256(HANDLE hFile) { } HMODULE VerifyAndLoadAnalyticsLibrary( - const wchar_t* library_filename, // This is expected to be just the DLL - // filename e.g. "analytics_win.dll" - const unsigned char* expected_hash, size_t expected_hash_size) { + const wchar_t* library_filename, + const std::vector>& allowed_hashes) { if (library_filename == nullptr || library_filename[0] == L'\0') { LogError(LOG_TAG "Invalid arguments."); return nullptr; } - if (expected_hash == nullptr || expected_hash_size == 0) { + if (allowed_hashes.empty()) { // Don't check the hash, just load the library. LogWarning(LOG_TAG "No hash provided, using unverified Analytics DLL."); return LoadLibraryW(library_filename); @@ -251,15 +250,23 @@ HMODULE VerifyAndLoadAnalyticsLibrary( if (calculated_hash.empty()) { LogError(LOG_TAG "Hash failed for Analytics DLL."); } else { - if (calculated_hash.size() != expected_hash_size) { - LogError(LOG_TAG - "Hash size mismatch for Analytics DLL. Expected: %zu, " - "Calculated: %zu.", - expected_hash_size, calculated_hash.size()); - } else if (memcmp(calculated_hash.data(), expected_hash, - expected_hash_size) != 0) { - LogError(LOG_TAG "Hash mismatch for Analytics DLL."); - } else { + bool hash_matched = false; + for (const auto& expected_hash : allowed_hashes) { + if (calculated_hash.size() != expected_hash.size()) { + LogVerbose(LOG_TAG + "Hash size mismatch for Analytics DLL. Expected: %zu, " + "Calculated: %zu. Trying next allowed hash.", + expected_hash.size(), calculated_hash.size()); + continue; + } + if (memcmp(calculated_hash.data(), expected_hash.data(), + expected_hash.size()) == 0) { + hash_matched = true; + break; + } + } + + if (hash_matched) { LogDebug(LOG_TAG "Successfully verified Analytics DLL."); // Load the library. When loading with a full path string, other // directories are not searched. @@ -269,6 +276,8 @@ HMODULE VerifyAndLoadAnalyticsLibrary( LogError(LOG_TAG "Library load failed for Analytics DLL. Error: %u", dwError); } + } else { + LogError(LOG_TAG "Hash mismatch for Analytics DLL."); } } diff --git a/analytics/src/analytics_windows.h b/analytics/src/analytics_windows.h index 4aecaaac4..f18683e02 100644 --- a/analytics/src/analytics_windows.h +++ b/analytics/src/analytics_windows.h @@ -17,13 +17,15 @@ #include +#include + namespace firebase { namespace analytics { namespace internal { -HMODULE VerifyAndLoadAnalyticsLibrary(const wchar_t* library_filename, - const unsigned char* expected_hash, - size_t expected_hash_size); +HMODULE VerifyAndLoadAnalyticsLibrary( + const wchar_t* library_filename, + const std::vector>& allowed_hashes); } // namespace internal } // namespace analytics From c6cf8f69edee310aa7a90537f9a6a1903b0ddca6 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sat, 14 Jun 2025 00:25:16 +0000 Subject: [PATCH 2/2] Refactor: Adjust log level for hash mismatch in analytics_windows I changed a LogVerbose call to LogDebug in `VerifyAndLoadAnalyticsLibrary` within `analytics/src/analytics_windows.cc`. The log message for a hash size mismatch when iterating through allowed hashes is now logged at Debug level instead of Verbose. --- analytics/src/analytics_windows.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/analytics/src/analytics_windows.cc b/analytics/src/analytics_windows.cc index 45c7aab85..2eb7ad0ef 100644 --- a/analytics/src/analytics_windows.cc +++ b/analytics/src/analytics_windows.cc @@ -253,10 +253,10 @@ HMODULE VerifyAndLoadAnalyticsLibrary( bool hash_matched = false; for (const auto& expected_hash : allowed_hashes) { if (calculated_hash.size() != expected_hash.size()) { - LogVerbose(LOG_TAG - "Hash size mismatch for Analytics DLL. Expected: %zu, " - "Calculated: %zu. Trying next allowed hash.", - expected_hash.size(), calculated_hash.size()); + LogDebug(LOG_TAG + "Hash size mismatch for Analytics DLL. Expected: %zu, " + "Calculated: %zu. Trying next allowed hash.", + expected_hash.size(), calculated_hash.size()); continue; } if (memcmp(calculated_hash.data(), expected_hash.data(),