Include loaded modules in safe browsing client incident reports.

This includes the sanitized paths of all loaded modules and an
indication for those that host active LSPs.

BUG=386156

Review URL: https://codereview.chromium.org/323953002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@278655 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/safe_browsing/environment_data_collection_win.cc

@@ -4,13 +4,94 @@
 
 #include "chrome/browser/safe_browsing/environment_data_collection_win.h"
 
+#include <windows.h>
+#include <set>
+
+#include "base/strings/string16.h"
+#include "base/strings/string_util.h"
+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/install_verification/win/module_info.h"
+#include "chrome/browser/install_verification/win/module_verification_common.h"
+#include "chrome/browser/net/service_providers_win.h"
+#include "chrome/browser/safe_browsing/path_sanitizer.h"
 #include "chrome/common/safe_browsing/csd.pb.h"
 
 namespace safe_browsing {
 
+namespace {
+
+// Helper function for expanding all environment variables in |path|.
+base::string16 ExpandEnvironmentVariables(const base::string16& path) {
+  static const DWORD kMaxBuffer = 32 * 1024;  // Max according to MSDN.
+  base::string16 path_expanded;
+  DWORD path_len = MAX_PATH;
+  do {
+    DWORD result = ExpandEnvironmentStrings(
+        path.c_str(), WriteInto(&path_expanded, path_len), path_len);
+    if (!result) {
+      // Failed to expand variables. Return the original string.
+      DPLOG(ERROR) << path;
+      break;
+    }
+    if (result <= path_len)
+      return path_expanded.substr(0, result - 1);
+    path_len = result;
+  } while (path_len < kMaxBuffer);
+
+  return path;
+}
+
+}  // namespace
+
+bool CollectDlls(ClientIncidentReport_EnvironmentData_Process* process) {
+  // Retrieve the module list.
+  std::set<ModuleInfo> loaded_modules;
+  if (!GetLoadedModules(&loaded_modules))
+    return false;
+
+  // Sanitize path of each module and add it to the incident report.
+  PathSanitizer path_sanitizer;
+  for (std::set<ModuleInfo>::const_iterator it = loaded_modules.begin();
+       it != loaded_modules.end();
+       ++it) {
+    base::FilePath dll_path(it->name);
+    path_sanitizer.StripHomeDirectory(&dll_path);
+
+    ClientIncidentReport_EnvironmentData_Process_Dll* dll = process->add_dll();
+    dll->set_path(base::WideToUTF8(dll_path.value()));
+    dll->set_base_address(it->base_address);
+    dll->set_length(it->size);
+  }
+
+  return true;
+}
+
+void RecordLspFeature(ClientIncidentReport_EnvironmentData_Process* process) {
+  WinsockLayeredServiceProviderList lsp_list;
+  GetWinsockLayeredServiceProviders(&lsp_list);
+
+  // For each LSP, we extract and sanitize the path.
+  PathSanitizer path_sanitizer;
+  std::set<base::string16> lsp_paths;
+  for (size_t i = 0; i < lsp_list.size(); ++i) {
+    base::FilePath lsp_path(ExpandEnvironmentVariables(lsp_list[i].path));
+    path_sanitizer.StripHomeDirectory(&lsp_path);
+    lsp_paths.insert(lsp_path.value());
+  }
+
+  // Look for a match between LSPs and loaded dlls.
+  for (int i = 0; i < process->dll_size(); ++i) {
+    if (lsp_paths.count(base::UTF8ToWide(process->dll(i).path()))) {
+      process->mutable_dll(i)
+          ->add_feature(ClientIncidentReport_EnvironmentData_Process_Dll::LSP);
+    }
+  }
+}
+
 void CollectPlatformProcessData(
     ClientIncidentReport_EnvironmentData_Process* process) {
-  // TODO(pmonette): collect dlls and lsps.
+  CollectDlls(process);
+  RecordLspFeature(process);
 }
 
 }  // namespace safe_browsing

chrome/browser/safe_browsing/environment_data_collection_win.h

@@ -7,7 +7,16 @@
 
 namespace safe_browsing {
 
-// TODO(pmonette): put DLL and LSP collection declarations here.
+class ClientIncidentReport_EnvironmentData_Process;
+
+// Collects then populates |process| with the sanitized paths of all DLLs
+// loaded in the current process. Return false if an error occurred while
+// querying for the loaded dlls.
+bool CollectDlls(ClientIncidentReport_EnvironmentData_Process* process);
+
+// For each of the dlls in this already populated incident report,
+// check one of them is a registered LSP.
+void RecordLspFeature(ClientIncidentReport_EnvironmentData_Process* process);
 
 }  // namespace safe_browsing
 

chrome/browser/safe_browsing/environment_data_collection_win_unittest.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/browser/safe_browsing/environment_data_collection_win.h"
+
+#include <string>
+
+#include "base/files/file_path.h"
+#include "base/scoped_native_library.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// Helper function that returns true if a dll with filename |dll_name| is
+// found in |process_report|.
+bool ProcessReportContainsDll(
+    const safe_browsing::ClientIncidentReport_EnvironmentData_Process&
+        process_report,
+    const base::FilePath& dll_name) {
+  for (int i = 0; i < process_report.dll_size(); ++i) {
+    base::FilePath current_dll =
+        base::FilePath::FromUTF8Unsafe(process_report.dll(i).path());
+
+    if (current_dll.BaseName() == dll_name)
+      return true;
+  }
+
+  return false;
+}
+
+}  // namespace
+
+TEST(SafeBrowsingEnvironmentDataCollectionWinTest, CollectDlls) {
+  // This test will check if the CollectDlls method works by loading
+  // a dll and then checking if we can find it within the process report.
+  // Pick msvidc32.dll as it is present from WinXP to Win8 and yet rarely used.
+  // msvidc32.dll exists in both 32 and 64 bit versions.
+  base::FilePath msvdc32_dll(L"msvidc32.dll");
+
+  safe_browsing::ClientIncidentReport_EnvironmentData_Process process_report;
+  safe_browsing::CollectDlls(&process_report);
+
+  ASSERT_FALSE(ProcessReportContainsDll(process_report, msvdc32_dll));
+
+  // Redo the same verification after loading a new dll.
+  base::ScopedNativeLibrary library(msvdc32_dll);
+
+  process_report.clear_dll();
+  safe_browsing::CollectDlls(&process_report);
+
+  ASSERT_TRUE(ProcessReportContainsDll(process_report, msvdc32_dll));
+}
+
+TEST(SafeBrowsingEnvironmentDataCollectionWinTest, RecordLspFeature) {
+  // Populate our incident report with loaded modules.
+  safe_browsing::ClientIncidentReport_EnvironmentData_Process process_report;
+  safe_browsing::CollectDlls(&process_report);
+
+  // We'll test RecordLspFeatures against a real dll registered as a LSP.
+  std::string lsp_path = "C:\\Windows\\system32\\mswsock.dll";
+  int base_address = 0x77770000;
+  int length = 0x180000;
+
+  // Look for dlls that are registered as a LSP.
+  safe_browsing::RecordLspFeature(&process_report);
+
+  // Look through dll entries and check that none contains the LSP feature.
+  bool lsp_feature_found = false;
+  for (int i = 0; i < process_report.dll_size(); ++i) {
+    if (process_report.dll(i).path() == lsp_path) {
+      // Look for ClientIncidentReport_EnvironmentData_Process_DLL_Feature_LSP
+      // through the features of each dll.
+      for (int j = 0; j < process_report.dll(i).feature_size(); ++j) {
+        if (process_report.dll(i).feature(j) ==
+            safe_browsing::ClientIncidentReport_EnvironmentData_Process_Dll::
+                LSP)
+          lsp_feature_found = true;
+      }
+    }
+  }
+
+  ASSERT_FALSE(lsp_feature_found);
+
+  // Manually add an entry to the process report that will get marked as a LSP.
+  safe_browsing::ClientIncidentReport_EnvironmentData_Process_Dll* dll =
+      process_report.add_dll();
+  dll->set_path(lsp_path);
+  dll->set_base_address(base_address);
+  dll->set_length(length);
+
+  // Look for dlls that are registered as a LSP.
+  safe_browsing::RecordLspFeature(&process_report);
+
+  // Look through dll entries and check if the one we added contains the LSP
+  // feature.
+  lsp_feature_found = false;
+  for (int i = 0; i < process_report.dll_size(); ++i) {
+    if (process_report.dll(i).path() == lsp_path) {
+      // Look for ClientIncidentReport_EnvironmentData_Process_DLL_Feature_LSP
+      // through the features of each dll.
+      for (int j = 0; j < process_report.dll(i).feature_size(); ++j) {
+        if (process_report.dll(i).feature(j) ==
+            safe_browsing::ClientIncidentReport_EnvironmentData_Process_Dll::
+                LSP)
+          lsp_feature_found = true;
+      }
+    }
+  }
+
+  ASSERT_TRUE(lsp_feature_found);
+}

chrome/browser/safe_browsing/path_sanitizer.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/browser/safe_browsing/path_sanitizer.h"
+
+#include "base/logging.h"
+#include "base/path_service.h"
+
+namespace safe_browsing {
+
+PathSanitizer::PathSanitizer() {
+  // Get the home directory path.
+  if (!PathService::Get(base::DIR_HOME, &home_path_))
+    NOTREACHED();
+}
+
+const base::FilePath& PathSanitizer::GetHomeDirectory() const {
+  return home_path_;
+}
+
+void PathSanitizer::StripHomeDirectory(base::FilePath* file_path) const {
+  base::FilePath sanitized_path(FILE_PATH_LITERAL("~"));
+
+  // The |file_path| is overwritten only if a relative path is found.
+  if (home_path_.AppendRelativePath(*file_path, &sanitized_path))
+    *file_path = sanitized_path;
+}
+
+}  // namespace safe_browsing

chrome/browser/safe_browsing/path_sanitizer.h

+// 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.
+
+#ifndef CHROME_BROWSER_SAFE_BROWSING_PATH_SANITIZER_H_
+#define CHROME_BROWSER_SAFE_BROWSING_PATH_SANITIZER_H_
+
+#include "base/files/file_path.h"
+#include "base/macros.h"
+
+namespace safe_browsing {
+
+// A helper class to sanitize any number of file paths by replacing the portion
+// that represents the current user's home directory with "~".
+class PathSanitizer {
+ public:
+  PathSanitizer();
+
+  const base::FilePath& GetHomeDirectory() const;
+
+  void StripHomeDirectory(base::FilePath* file_path) const;
+
+ private:
+  base::FilePath home_path_;
+
+  DISALLOW_COPY_AND_ASSIGN(PathSanitizer);
+};
+
+}  // namespace safe_browsing
+
+#endif  // CHROME_BROWSER_SAFE_BROWSING_PATH_SANITIZER_H_

chrome/browser/safe_browsing/path_sanitizer_unittest.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/browser/safe_browsing/path_sanitizer.h"
+
+#include <vector>
+
+#include "base/logging.h"
+#include "base/path_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+// Returns the root directory with a trailing separator. Works on all platforms.
+base::FilePath GetRootDirectory() {
+  base::FilePath dir_temp;
+  if (!PathService::Get(base::DIR_TEMP, &dir_temp))
+    NOTREACHED();
+
+  std::vector<base::FilePath::StringType> components;
+  dir_temp.GetComponents(&components);
+
+  return base::FilePath(components[0]).AsEndingWithSeparator();
+}
+
+}  // namespace
+
+TEST(SafeBrowsingPathSanitizerTest, HomeDirectoryIsNotEmpty) {
+  safe_browsing::PathSanitizer path_sanitizer;
+
+  ASSERT_FALSE(path_sanitizer.GetHomeDirectory().empty());
+}
+
+TEST(SafeBrowsingPathSanitizerTest, DontStripHomeDirectoryTest) {
+  // Test with path not in home directory.
+  base::FilePath path =
+      GetRootDirectory().Append(FILE_PATH_LITERAL("not_in_home_directory.ext"));
+  base::FilePath path_expected = path;
+
+  safe_browsing::PathSanitizer path_sanitizer;
+  path_sanitizer.StripHomeDirectory(&path);
+
+  ASSERT_EQ(path.value(), path_expected.value());
+}
+
+TEST(SafeBrowsingPathSanitizerTest, DoStripHomeDirectoryTest) {
+  // Test with path in home directory.
+  safe_browsing::PathSanitizer path_sanitizer;
+
+  base::FilePath path = path_sanitizer.GetHomeDirectory().Append(
+      FILE_PATH_LITERAL("in_home_directory.ext"));
+  base::FilePath path_expected = base::FilePath(FILE_PATH_LITERAL("~")).Append(
+      FILE_PATH_LITERAL("in_home_directory.ext"));
+
+  path_sanitizer.StripHomeDirectory(&path);
+
+  ASSERT_EQ(path.value(), path_expected.value());
+}

chrome/chrome_browser.gypi

@@ -1876,6 +1876,8 @@
         'browser/safe_browsing/malware_details_cache.h',
         'browser/safe_browsing/malware_details_history.cc',
         'browser/safe_browsing/malware_details_history.h',
+        'browser/safe_browsing/path_sanitizer.cc',
+        'browser/safe_browsing/path_sanitizer.h',
         'browser/safe_browsing/pe_image_reader_win.cc',
         'browser/safe_browsing/pe_image_reader_win.h',
         'browser/safe_browsing/ping_manager.cc',

chrome/chrome_tests_unit.gypi

@@ -1249,10 +1249,12 @@
         'browser/safe_browsing/download_feedback_service_unittest.cc',
         'browser/safe_browsing/download_feedback_unittest.cc',
         'browser/safe_browsing/download_protection_service_unittest.cc',
+        'browser/safe_browsing/environment_data_collection_win_unittest.cc',
         'browser/safe_browsing/incident_report_uploader_impl_unittest.cc',
         'browser/safe_browsing/incident_reporting_service_unittest.cc',
         'browser/safe_browsing/local_two_phase_testserver.cc',
         'browser/safe_browsing/malware_details_unittest.cc',
+        'browser/safe_browsing/path_sanitizer_unittest.cc',
         'browser/safe_browsing/pe_image_reader_win_unittest.cc',
         'browser/safe_browsing/ping_manager_unittest.cc',
         'browser/safe_browsing/preference_validation_delegate_unittest.cc',

chrome/common/safe_browsing/csd.proto

@@ -110,7 +110,7 @@ message ClientMalwareRequest {
     required string url = 2;
     optional string method = 3;
     optional string referrer = 4;
-    // Resource type, the int value is a direct cast from the Type enum 
+    // Resource type, the int value is a direct cast from the Type enum
     // of ResourceType class defined in //src/webkit/commom/resource_type.h
     optional int32 resource_type = 5;
   }
@@ -380,7 +380,7 @@ message ClientIncidentReport {
     optional Machine machine = 2;
     message Process {
       optional string version = 1;
-      repeated string dlls = 2;
+      repeated string OBSOLETE_dlls = 2;
       message Patch {
         optional string function = 1;
         optional string target_dll = 2;
@@ -399,6 +399,17 @@ message ClientIncidentReport {
       optional int64 uptime_msec = 6;
       optional bool metrics_consent = 7;
       optional bool extended_consent = 8;
+      message Dll {
+        enum Feature {
+          UNKNOWN = 0;
+          LSP = 1;
+        }
+        optional string path = 1;
+        optional uint64 base_address = 2;
+        optional uint32 length = 3;
+        repeated Feature feature = 4;
+      }
+      repeated Dll dll = 9;
     }
     optional Process process = 3;
   }