Add a ModuleLoadAnalyzer which checks modules against a whitelist

chrome/browser/safe_browsing/incident_reporting/module_load_analyzer_win.cc

+// Copyright 2016 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/incident_reporting/module_load_analyzer.h"
+
+#include <set>
+#include <utility>
+
+#include "base/file_version_info.h"
+#include "base/files/file_path.h"
+#include "base/i18n/case_conversion.h"
+#include "base/logging.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/strings/string16.h"
+#include "base/strings/string_number_conversions.h"

[grt (UTC plus 2), 2016/02/15 16:46:50]

unused?

[proberge, 2016/02/16 16:56:23]

Done.

+#include "base/strings/string_util.h"

[grt (UTC plus 2), 2016/02/15 16:46:50]

is this used?

[proberge, 2016/02/16 16:56:23]

Done.

+#include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"

[grt (UTC plus 2), 2016/02/15 16:46:50]

unused?

[proberge, 2016/02/16 16:56:23]

Done.

+#include "chrome/browser/install_verification/win/module_info.h"
+#include "chrome/browser/install_verification/win/module_verification_common.h"
+#include "chrome/browser/safe_browsing/incident_reporting/incident_receiver.h"
+#include "chrome/browser/safe_browsing/incident_reporting/suspicious_module_incident.h"
+#include "chrome/browser/safe_browsing/path_sanitizer.h"
+#include "chrome/browser/safe_browsing/safe_browsing_service.h"

[grt (UTC plus 2), 2016/02/15 16:46:50]

unused?

[proberge, 2016/02/16 16:56:23]

Done.

+#include "chrome/common/safe_browsing/binary_feature_extractor.h"
+#include "chrome/common/safe_browsing/csd.pb.h"
+
+#if defined(SAFE_BROWSING_DB_LOCAL)
+#include "chrome/browser/safe_browsing/local_database_manager.h"
+#elif defined(SAFE_BROWSING_DB_REMOTE)
+#include "chrome/browser/safe_browsing/remote_database_manager.h"
+#endif
+
+namespace safe_browsing {
+
+namespace {
+
+void ReportIncidentsForSuspiciousModules(
+    scoped_ptr<std::set<base::FilePath>> module_names,

[grt (UTC plus 2), 2016/02/15 16:46:50]

module_names -> module_paths

[proberge, 2016/02/16 16:56:23]

Done.

+    scoped_ptr<IncidentReceiver> incident_receiver) {
+  PathSanitizer path_sanitizer;
+  scoped_refptr<BinaryFeatureExtractor> binary_feature_extractor(
+      new BinaryFeatureExtractor());
+  SCOPED_UMA_HISTOGRAM_TIMER("SBIRS.SuspiciousModuleReportingTime");
+
+  for (const auto& module_name : *module_names) {

[grt (UTC plus 2), 2016/02/15 16:46:50]

module_name -> module_path and delete lines 54 and 55

[proberge, 2016/02/16 16:56:23]

Done.

+    // TODO(proberge): Skip over modules that have already been reported.

[grt (UTC plus 2), 2016/02/15 16:46:50]

how hard do you want to work at this? do you desire to do the skipping here for
the sake of efficiency rather than let the service's pruning take care of it for
you? given the way that SuspiciousModuleIncident::ComputeDigest() is
implemented, pruning is done based on the full payload of the incident, so all
of this data needs to be collected anyway. if you like, you could change it so
that the digest is a constant so that modules of a given path are reported at
most once. in this case, you could bounce the list of paths to the UI thread and
in some way call HasBeenReported before extracting their features.

[proberge, 2016/02/16 16:56:23]

My main concern was that a Profile/ProfileContext is needed to read the
StateStore from, which we don't know about at this point. 
Would prefer to do in a future CL, if at all, to minimize complexity in this
one.

[grt (UTC plus 2), 2016/02/17 18:04:59]

Ack

+
+    scoped_ptr<ClientIncidentReport_IncidentData_SuspiciousModuleIncident>
+        suspicious_module(
+            new ClientIncidentReport_IncidentData_SuspiciousModuleIncident());
+
+    const base::FilePath module_path(module_name);
+
+    // Sanitized path.
+    base::FilePath sanitized_path(module_path);
+    path_sanitizer.StripHomeDirectory(&sanitized_path);
+    suspicious_module->set_path(base::WideToUTF8(sanitized_path.value()));

[grt (UTC plus 2), 2016/02/15 16:46:50]

base::WideToUTF8(sanitized_path.value()) -> sanitized_path.AsUTF8Unsafe()

[proberge, 2016/02/16 16:56:23]

Done.

+
+    // Digest.
+    binary_feature_extractor->ExtractDigest(
+        module_path, suspicious_module->mutable_digest());
+
+    // Version.
+    scoped_ptr<FileVersionInfo> version_info(
+        FileVersionInfo::CreateFileVersionInfo(module_path));
+    if (version_info) {
+      base::string16 file_version = version_info->file_version();
+      if (!file_version.empty())
+        suspicious_module->set_version(base::WideToUTF8(file_version));

[grt (UTC plus 2), 2016/02/15 16:46:50]

base::UTF16ToUTF8

[proberge, 2016/02/16 16:56:23]

Done.

+    }
+
+    // Signature.
+    binary_feature_extractor->CheckSignature(
+        module_path, suspicious_module->mutable_signature());
+
+    // Image headers.
+    if (!binary_feature_extractor->ExtractImageFeatures(
+            module_path, BinaryFeatureExtractor::kDefaultOptions,
+            suspicious_module->mutable_image_headers(),
+            nullptr /* signed_data */)) {
+      suspicious_module->clear_image_headers();
+    }
+
+    // Send the incident to the reporting service.
+    incident_receiver->AddIncidentForProcess(make_scoped_ptr(
+        new SuspiciousModuleIncident(std::move(suspicious_module))));
+  }
+}
+
+void CheckModuleWhitelistOnIOThread(
+    const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
+    scoped_ptr<IncidentReceiver> incident_receiver,
+    scoped_ptr<std::set<ModuleInfo>> module_info_set) {
+  SCOPED_UMA_HISTOGRAM_TIMER("SBIRS.SuspiciousModuleDetectionTime");
+  scoped_ptr<std::set<base::FilePath>> suspicious_names(

[grt (UTC plus 2), 2016/02/15 16:46:50]

nit: this contains the full paths, not just the names. suspicious_paths seems
more clear.

[proberge, 2016/02/16 16:56:23]

Done.

+      new std::set<base::FilePath>);
+
+  for (const ModuleInfo& module_info : *module_info_set) {
+    base::string16 module_file_name(base::i18n::FoldCase(
+        base::FilePath(module_info.name).BaseName().value()));

[grt (UTC plus 2), 2016/02/15 16:46:50]

nit: stuff base::FilePath(module_info.name) in a local var so that it isn't
created twice

[grt (UTC plus 2), 2016/02/15 16:46:50]

.value() -> .AsUTF16Unsafe()

[proberge, 2016/02/16 16:56:23]

Done.

[proberge, 2016/02/16 16:56:23]

Done.

+
+    // If not whitelisted.
+    if (!database_manager->MatchModuleWhitelistString(
+            base::UTF16ToUTF8(module_file_name)))
+      suspicious_names->insert(base::FilePath(module_info.name));
+  }
+
+  UMA_HISTOGRAM_COUNTS("SBIRS.SuspiciousModuleReportCount",
+                       suspicious_names->size());
+
+  if (!suspicious_names->empty()) {
+    content::BrowserThread::GetBlockingPool()

[grt (UTC plus 2), 2016/02/15 16:46:50]

#include "content/public/browser/browser_thread.h"

[proberge, 2016/02/16 16:56:23]

Done.

+        ->PostWorkerTaskWithShutdownBehavior(
+            FROM_HERE, base::Bind(&ReportIncidentsForSuspiciousModules,
+                                  base::Passed(std::move(suspicious_names)),
+                                  base::Passed(std::move(incident_receiver))),
+            base::SequencedWorkerPool::CONTINUE_ON_SHUTDOWN);
+  }
+}
+
+}  // namespace
+
+void VerifyModuleLoadState(
+    const scoped_refptr<SafeBrowsingDatabaseManager>& database_manager,
+    scoped_ptr<IncidentReceiver> incident_receiver) {
+  scoped_ptr<std::set<ModuleInfo>> module_info_set(new std::set<ModuleInfo>);
+  if (!GetLoadedModules(module_info_set.get()))
+    return;
+
+  // PostTaskAndReply doesn't work here because we're in a sequenced blocking
+  // thread pool.
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::Bind(&CheckModuleWhitelistOnIOThread, database_manager,
+                 base::Passed(std::move(incident_receiver)),
+                 base::Passed(std::move(module_info_set))));
+}
+
+}  // namespace safe_browsing