Add a better shell extension enumeration behind a feature

Check where the shell extensions are actually registered instead of only
looking at the list of Approved shell extensions.

Change-Id: I3257106060f4775fe83f8af322c007e022064630
Reviewed-on: https://chromium-review.googlesource.com/1197352Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590482}

chrome/browser/conflicts/enumerate_shell_extensions_win.cc

@@ -8,7 +8,6 @@
 
 #include "base/files/file_path.h"
 #include "base/memory/ref_counted.h"
-#include "base/metrics/histogram_macros.h"
 #include "base/sequenced_task_runner.h"
 #include "base/strings/string16.h"
 #include "base/strings/stringprintf.h"
@@ -19,31 +18,168 @@
 #include "base/win/registry.h"
 #include "chrome/browser/conflicts/module_info_util_win.h"
 
+// The following link explains how Shell extensions are registered on Windows.
+// Different types of handlers can be applied to different types of Shell
+// objects.
+// https://docs.microsoft.com/en-us/windows/desktop/shell/reg-shell-exts
+
 namespace {
 
+// The different kinds of shell objects that can be affected by a shell
+// extension.
+constexpr wchar_t kEverything[] = L"*";
+constexpr wchar_t kAllFileSystemObjects[] = L"AllFileSystemObjects";
+constexpr wchar_t kDesktopBackground[] = L"DesktopBackground";
+constexpr wchar_t kDirectory[] = L"Directory";
+constexpr wchar_t kDirectoryBackground[] = L"Directory\\Background";
+constexpr wchar_t kDrive[] = L"Drive";
+constexpr wchar_t kFolder[] = L"Folder";
+constexpr wchar_t kNetServer[] = L"NetServer";
+constexpr wchar_t kNetShare[] = L"NetShare";
+constexpr wchar_t kNetwork[] = L"Network";
+constexpr wchar_t kPrinters[] = L"Printers";
+
+// Retrieves the path to the registry key that contains all the shell extensions
+// of type |shell_extension_type| that apply to |shell_object_type|.
+base::string16 GetShellExtensionTypePath(const wchar_t* shell_extension_type,
+                                         const wchar_t* shell_object_type) {
+  return base::StringPrintf(L"%ls\\shellex\\%ls", shell_object_type,
+                            shell_extension_type);
+}
+
+// Returns the path to the DLL for an InProcServer32 registration.
+base::FilePath GetInProcServerPath(const wchar_t* guid) {
+  base::string16 key = base::StringPrintf(kClassIdRegistryKeyFormat, guid);
+
+  base::win::RegKey clsid;
+  if (clsid.Open(HKEY_CLASSES_ROOT, key.c_str(), KEY_QUERY_VALUE) !=
+      ERROR_SUCCESS) {
+    return base::FilePath();
+  }
+
+  base::string16 dll_path;
+  if (clsid.ReadValue(L"", &dll_path) != ERROR_SUCCESS)
+    return base::FilePath();
+
+  return base::FilePath(std::move(dll_path));
+}
+
+// Reads all the shell extensions of type |shell_extension_type| that are
+// applied to |shell_object_types| and forwards them to |callback|.
 void ReadShellExtensions(
-    HKEY parent,
-    const base::RepeatingCallback<void(const base::FilePath&)>& callback,
-    int* nb_shell_extensions) {
-  for (base::win::RegistryValueIterator iter(parent,
-                                             kShellExtensionRegistryKey);
+    const wchar_t* shell_extension_type,
+    const wchar_t* shell_object_type,
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  base::string16 path =
+      GetShellExtensionTypePath(shell_extension_type, shell_object_type);
+
+  DCHECK_NE(path.back(), L'\\');
+
+  base::string16 guid;
+  for (base::win::RegistryKeyIterator iter(HKEY_CLASSES_ROOT, path.c_str());
        iter.Valid(); ++iter) {
-    base::string16 key =
-        base::StringPrintf(kClassIdRegistryKeyFormat, iter.Name());
+    base::string16 shell_extension_reg_path = path + L"\\" + iter.Name();
+    base::win::RegKey reg_key(
+        HKEY_CLASSES_ROOT, shell_extension_reg_path.c_str(), KEY_QUERY_VALUE);
+    if (!reg_key.Valid())
+      continue;
 
-    base::win::RegKey clsid;
-    if (clsid.Open(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ) != ERROR_SUCCESS)
+    guid.clear();
+    reg_key.ReadValue(nullptr, &guid);
+    if (guid.empty())
       continue;
 
-    base::string16 dll;
-    if (clsid.ReadValue(L"", &dll) != ERROR_SUCCESS)
+    base::FilePath shell_extension_path = GetInProcServerPath(guid.c_str());
+    if (shell_extension_path.empty())
       continue;
 
-    (*nb_shell_extensions)++;
-    callback.Run(base::FilePath(dll));
+    callback.Run(shell_extension_path);
   }
 }
 
+// Reads all the shell extensions that are in the Approved list and forwards
+// them to |callback|.
+void ReadApprovedShellExtensions(
+    HKEY parent,
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  for (base::win::RegistryValueIterator iter(
+           parent, kApprovedShellExtensionRegistryKey);
+       iter.Valid(); ++iter) {
+    // Skip the key's default value.
+    if (!*iter.Name())
+      continue;
+
+    base::FilePath shell_extension_path = GetInProcServerPath(iter.Name());
+    if (shell_extension_path.empty())
+      continue;
+
+    callback.Run(shell_extension_path);
+  }
+}
+
+// Reads all the shell extensions of type ColumnHandlers.
+void ReadColumnHandlers(
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  // Column handlers can only be applied to folders.
+  ReadShellExtensions(L"ColumnHandlers", kFolder, callback);
+}
+
+// Reads all the shell extensions of type CopyHookHandlers.
+void ReadCopyHookHandlers(
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  static constexpr const wchar_t* kSupportedShellObjects[] = {
+      kDirectory, kPrinters,
+  };
+
+  for (const auto* shell_object : kSupportedShellObjects)
+    ReadShellExtensions(L"CopyHookHandlers", shell_object, callback);
+}
+
+// Reads all the shell extensions of type DragDropHandlers.
+void ReadDragDropHandlers(
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  static constexpr const wchar_t* kSupportedShellObjects[] = {
+      kDirectory, kDrive, kFolder,
+  };
+
+  for (const auto* shell_object : kSupportedShellObjects)
+    ReadShellExtensions(L"DragDropHandlers", shell_object, callback);
+}
+
+// Reads all the shell extensions of type ContextMenuHandlers and
+// PropertySheetHandlers.
+void ReadContextMenuAndPropertySheetHandlers(
+    const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
+  static constexpr const wchar_t* kHandlerTypes[] = {
+      L"ContextMenuHandlers", L"PropertySheetHandlers",
+  };
+
+  // This list is not exhaustive and does not cover cases where a shell
+  // extension is installed for a specific file extension or a specific
+  // executable. It should still pick up all the general-purpose shell
+  // extensions.
+  static constexpr const wchar_t* kSupportedShellObjects[] = {
+      kEverything,
+      kAllFileSystemObjects,
+      kFolder,
+      kDirectory,
+      kDirectoryBackground,
+      kDesktopBackground,
+      kDrive,
+      kNetwork,
+      kNetShare,
+      kNetServer,
+      kPrinters,
+  };
+
+  for (const auto* handler_type : kHandlerTypes) {
+    for (const auto* shell_object : kSupportedShellObjects)
+      ReadShellExtensions(handler_type, shell_object, callback);
+  }
+}
+
+// Retrieves the module size and time date stamp for the shell extension and
+// forwards it to the callback on |task_runner|.
 void OnShellExtensionPathEnumerated(
     scoped_refptr<base::SequencedTaskRunner> task_runner,
     OnShellExtensionEnumeratedCallback on_shell_extension_enumerated,
@@ -73,9 +209,12 @@ void EnumerateShellExtensionsOnBlockingSequence(
 
 }  // namespace
 
-const wchar_t kShellExtensionRegistryKey[] =
+const wchar_t kApprovedShellExtensionRegistryKey[] =
     L"Software\\Microsoft\\Windows\\CurrentVersion\\Shell Extensions\\Approved";
 
+const base::Feature kExtendedShellExtensionsEnumeration{
+    "ExtendedShellExtensionsEnumeration", base::FEATURE_DISABLED_BY_DEFAULT};
+
 void EnumerateShellExtensions(
     OnShellExtensionEnumeratedCallback on_shell_extension_enumerated,
     base::OnceClosure on_enumeration_finished) {
@@ -95,12 +234,15 @@ void EnumerateShellExtensionPaths(
     const base::RepeatingCallback<void(const base::FilePath&)>& callback) {
   base::ScopedBlockingCall scoped_blocking_call(base::BlockingType::MAY_BLOCK);
 
-  int nb_shell_extensions = 0;
-  ReadShellExtensions(HKEY_LOCAL_MACHINE, callback, &nb_shell_extensions);
-  ReadShellExtensions(HKEY_CURRENT_USER, callback, &nb_shell_extensions);
+  ReadApprovedShellExtensions(HKEY_LOCAL_MACHINE, callback);
+  ReadApprovedShellExtensions(HKEY_CURRENT_USER, callback);
 
-  UMA_HISTOGRAM_COUNTS_100("ThirdPartyModules.ShellExtensionsCount2",
-                           nb_shell_extensions);
+  if (base::FeatureList::IsEnabled(kExtendedShellExtensionsEnumeration)) {
+    ReadColumnHandlers(callback);
+    ReadCopyHookHandlers(callback);
+    ReadDragDropHandlers(callback);
+    ReadContextMenuAndPropertySheetHandlers(callback);
+  }
 }
 
 }  // namespace internal

chrome/browser/conflicts/enumerate_shell_extensions_win.h

@@ -8,13 +8,18 @@
 #include <stdint.h>
 
 #include "base/callback_forward.h"
+#include "base/feature_list.h"
 
 namespace base {
 class FilePath;
 }
 
 // The path to the registry key where shell extensions are registered.
-extern const wchar_t kShellExtensionRegistryKey[];
+extern const wchar_t kApprovedShellExtensionRegistryKey[];
+
+// This feature controls whether additional locations are enumerated to find
+// shell extensions in the registry.
+extern const base::Feature kExtendedShellExtensionsEnumeration;
 
 // Finds shell extensions installed on the computer by enumerating the registry.
 // In addition to the file path, the SizeOfImage and TimeDateStamp of the module

chrome/browser/conflicts/enumerate_shell_extensions_win_unittest.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/conflicts/enumerate_shell_extensions_win.h"
 
-#include <tuple>
 #include <vector>
 
 #include "base/bind.h"
@@ -12,6 +11,7 @@
 #include "base/path_service.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_environment.h"
 #include "base/test/test_reg_util_win.h"
 #include "chrome/browser/conflicts/module_info_util_win.h"
@@ -48,9 +48,9 @@ class EnumerateShellExtensionsTest : public testing::Test {
 // Adds a fake shell extension entry to the registry that should be found by
 // the ShellExtensionEnumerator. The call must be wrapped inside an
 // ASSERT_NO_FATAL_FAILURE.
-void RegisterFakeShellExtension(HKEY key,
-                                const wchar_t* guid,
-                                const wchar_t* path) {
+void RegisterFakeApprovedShellExtension(HKEY key,
+                                        const wchar_t* guid,
+                                        const wchar_t* path) {
   base::win::RegKey class_id(
       HKEY_CLASSES_ROOT,
       base::StringPrintf(kClassIdRegistryKeyFormat, guid).c_str(), KEY_WRITE);
@@ -58,10 +58,34 @@ void RegisterFakeShellExtension(HKEY key,
 
   ASSERT_EQ(ERROR_SUCCESS, class_id.WriteValue(nullptr, path));
 
-  base::win::RegKey registration(key, kShellExtensionRegistryKey, KEY_WRITE);
+  base::win::RegKey registration(key, kApprovedShellExtensionRegistryKey,
+                                 KEY_WRITE);
   ASSERT_EQ(ERROR_SUCCESS, registration.WriteValue(guid, L""));
 }
 
+// Adds a fake shell extension entry to the registry that should be found by
+// the ShellExtensionEnumerator. The call must be wrapped inside an
+// ASSERT_NO_FATAL_FAILURE.
+void RegisterFakeShellExtension(const wchar_t* guid,
+                                const wchar_t* path,
+                                const wchar_t* shell_extension_type,
+                                const wchar_t* shell_object_type) {
+  base::win::RegKey class_id(
+      HKEY_CLASSES_ROOT,
+      base::StringPrintf(kClassIdRegistryKeyFormat, guid).c_str(), KEY_WRITE);
+  ASSERT_TRUE(class_id.Valid());
+
+  ASSERT_EQ(ERROR_SUCCESS, class_id.WriteValue(nullptr, path));
+
+  base::win::RegKey registration(
+      HKEY_CLASSES_ROOT,
+      base::StringPrintf(L"%ls\\shellex\\%ls\\&ls", shell_object_type,
+                         shell_extension_type, guid)
+          .c_str(),
+      KEY_WRITE);
+  ASSERT_EQ(ERROR_SUCCESS, registration.WriteValue(nullptr, guid));
+}
+
 void OnShellExtensionPathEnumerated(
     std::vector<base::FilePath>* shell_extension_paths,
     const base::FilePath& path) {
@@ -82,20 +106,23 @@ void OnEnumerationFinished(bool* is_enumeration_finished) {
 
 }  // namespace
 
-// Registers a few fake shell extensions then see if
+// Registers a few fake approved shell extensions then see if
 // EnumerateShellExtensionPaths() finds them.
-TEST_F(EnumerateShellExtensionsTest, EnumerateShellExtensionPaths) {
-  const std::tuple<HKEY, const wchar_t*, const wchar_t*> test_cases[] = {
+TEST_F(EnumerateShellExtensionsTest, EnumerateApprovedShellExtensionPaths) {
+  struct {
+    HKEY hkey_root;
+    const wchar_t* guid;
+    const wchar_t* path;
+  } kTestCases[] = {
       {HKEY_CURRENT_USER, L"{FAKE_GUID_0001}", L"c:\\module.dll"},
       {HKEY_LOCAL_MACHINE, L"{FAKE_GUID_0002}", L"c:\\dir\\shell_ext.dll"},
       {HKEY_LOCAL_MACHINE, L"{FAKE_GUID_0003}", L"c:\\path\\test.dll"},
   };
 
-  // Register all fake shell extensions in test_cases.
-  for (const auto& test_case : test_cases) {
-    ASSERT_NO_FATAL_FAILURE(RegisterFakeShellExtension(std::get<0>(test_case),
-                                                       std::get<1>(test_case),
-                                                       std::get<2>(test_case)));
+  // Register all fake shell extensions in kTestCases.
+  for (const auto& test_case : kTestCases) {
+    ASSERT_NO_FATAL_FAILURE(RegisterFakeApprovedShellExtension(
+        test_case.hkey_root, test_case.guid, test_case.path));
   }
 
   std::vector<base::FilePath> shell_extension_paths;
@@ -104,18 +131,18 @@ TEST_F(EnumerateShellExtensionsTest, EnumerateShellExtensionPaths) {
                           base::Unretained(&shell_extension_paths)));
 
   ASSERT_EQ(3u, shell_extension_paths.size());
-  for (size_t i = 0; i < arraysize(test_cases); i++) {
+  for (size_t i = 0; i < base::size(kTestCases); i++) {
     // The inefficiency is fine as long as the number of test cases stays small.
-    EXPECT_TRUE(base::ContainsValue(
-        shell_extension_paths, base::FilePath(std::get<2>(test_cases[i]))));
+    EXPECT_TRUE(base::ContainsValue(shell_extension_paths,
+                                    base::FilePath(kTestCases[i].path)));
   }
 }
 
-TEST_F(EnumerateShellExtensionsTest, EnumerateShellExtensions) {
+TEST_F(EnumerateShellExtensionsTest, EnumerateApprovedShellExtensions) {
   // Use the current exe file as an arbitrary module that exists.
   base::FilePath file_exe;
   ASSERT_TRUE(base::PathService::Get(base::FILE_EXE, &file_exe));
-  ASSERT_NO_FATAL_FAILURE(RegisterFakeShellExtension(
+  ASSERT_NO_FATAL_FAILURE(RegisterFakeApprovedShellExtension(
       HKEY_LOCAL_MACHINE, L"{FAKE_GUID}", file_exe.value().c_str()));
 
   std::vector<base::FilePath> shell_extension_paths;
@@ -132,3 +159,43 @@ TEST_F(EnumerateShellExtensionsTest, EnumerateShellExtensions) {
   ASSERT_EQ(1u, shell_extension_paths.size());
   EXPECT_EQ(file_exe, shell_extension_paths[0]);
 }
+
+TEST_F(EnumerateShellExtensionsTest, EnumerateShellExtensionPaths) {
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeature(kExtendedShellExtensionsEnumeration);
+
+  struct {
+    const wchar_t* guid;
+    const wchar_t* path;
+    const wchar_t* shell_extension_type;
+    const wchar_t* shell_object_type;
+  } kTestCases[] = {
+      {L"{FAKE_GUID_0001}", L"c:\\module.dll", L"ColumnHandlers", L"Folder"},
+      {L"{FAKE_GUID_0002}", L"c:\\dir\\shell_ext.dll", L"ContextMenuHandlers",
+       L"*"},
+      {L"{FAKE_GUID_0003}", L"c:\\path\\test.dll", L"CopyHookHandlers",
+       L"Printers"},
+      {L"{FAKE_GUID_0004}", L"c:\\foo\\bar.dll", L"DragDropHandlers", L"Drive"},
+      {L"{FAKE_GUID_0005}", L"c:\\foo\\baz.dll", L"PropertySheetHandlers",
+       L"AllFileSystemObjects"},
+  };
+
+  // Register all fake shell extensions in kTestCases.
+  for (const auto& test_case : kTestCases) {
+    ASSERT_NO_FATAL_FAILURE(RegisterFakeShellExtension(
+        test_case.guid, test_case.path, test_case.shell_extension_type,
+        test_case.shell_object_type));
+  }
+
+  std::vector<base::FilePath> shell_extension_paths;
+  internal::EnumerateShellExtensionPaths(
+      base::BindRepeating(&OnShellExtensionPathEnumerated,
+                          base::Unretained(&shell_extension_paths)));
+
+  ASSERT_EQ(5u, shell_extension_paths.size());
+  for (size_t i = 0; i < base::size(kTestCases); ++i) {
+    // The inefficiency is fine as long as the number of test cases stays small.
+    EXPECT_TRUE(base::ContainsValue(shell_extension_paths,
+                                    base::FilePath(kTestCases[i].path)));
+  }
+}

chrome/browser/conflicts/third_party_metrics_recorder_win.cc

@@ -66,6 +66,9 @@ void ThirdPartyMetricsRecorder::OnNewModuleFound(
     if (module_data.module_properties & ModuleInfoData::kPropertyLoadedModule)
       AddUnsignedModuleToCrashkeys(module_data.inspection_result->basename);
   }
+
+  if (module_data.module_properties & ModuleInfoData::kPropertyShellExtension)
+    shell_extensions_count_++;
 }
 
 void ThirdPartyMetricsRecorder::OnModuleDatabaseIdle() {
@@ -89,6 +92,9 @@ void ThirdPartyMetricsRecorder::OnModuleDatabaseIdle() {
                                  module_count_, 1, 500, 50);
   base::UmaHistogramCustomCounts("ThirdPartyModules.Modules.Unsigned",
                                  unsigned_module_count_, 1, 500, 50);
+
+  base::UmaHistogramCounts100("ThirdPartyModules.ShellExtensionsCount3",
+                              shell_extensions_count_);
 }
 
 void ThirdPartyMetricsRecorder::AddUnsignedModuleToCrashkeys(

chrome/browser/conflicts/third_party_metrics_recorder_win.h

@@ -54,6 +54,9 @@ class ThirdPartyMetricsRecorder : public ModuleDatabaseObserver {
   size_t loaded_third_party_module_count_ = 0;
   size_t not_loaded_third_party_module_count_ = 0;
 
+  // Counts the number of shell extensions.
+  size_t shell_extensions_count_ = 0;
+
   DISALLOW_COPY_AND_ASSIGN(ThirdPartyMetricsRecorder);
 };
 

tools/metrics/histograms/histograms.xml

@@ -107322,6 +107322,9 @@ uploading your change for review.
 </histogram>
 
 <histogram name="ThirdPartyModules.ShellExtensionsCount2" units="counts">
+  <obsolete>
+    Deprecated on 30/08/2018 now that the enumeration removes duplicate entries.
+  </obsolete>
   <owner>pmonette@chromium.org</owner>
   <summary>
     The number of registered shell extensions found on the user's machine. This
@@ -107330,6 +107333,15 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="ThirdPartyModules.ShellExtensionsCount3" units="counts">
+  <owner>pmonette@chromium.org</owner>
+  <summary>
+    The number of registered shell extensions found on the user's machine. This
+    is emitted shortly after startup when the shell extensions enumeration is
+    done. Doesn't count duplicates.
+  </summary>
+</histogram>
+
 <histogram name="ThirdPartyModules.TimeDateStampObtained" enum="BooleanSuccess">
   <owner>pmonette@chromium.org</owner>
   <summary>