Prevent loading extensions when the ID in the preferences does not match the extension's real ID.

BUG=394410
TBR=laforge@chromium.org,asargent@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#331488}
(cherry picked from commit 55a41a677015add974720eb87544d35cd9fd942a)

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

Cr-Commit-Position: refs/branch-heads/2357@{#445}
Cr-Branched-From: 59d4494849b405682265ed5d3f5164573b9a939b-refs/heads/master@{#323860}

3 files changed, 15 insertions, 45 deletions

diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index 43f3154..7025ec8 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -196,7 +196,6 @@ const char page_action[] = "obcimlgaoabeegjmmpldobjndiealpln";
 const char theme_crx[] = "iamefpfkojoapidjnbafmgkgncegbkad";
 const char theme2_crx[] = "pjpgmfcmabopnnfonnhmdjglfpjjfkbf";
 const char permissions_crx[] = "eagpmdpfmaekmmcejjbmjoecnejeiiin";
-const char unpacked[] = "cbcdidchbppangcjoddlpdjlenngjldk";
 const char updates_from_webstore[] = "akjooamlhcgeopfifcmlggaebeocgokj";
 const char permissions_blocklist[] = "noffkehfcaggllbcojjbopcmlhcnhcdn";
 
@@ -2325,49 +2324,6 @@ TEST_F(ExtensionServiceTest, LoadLocalizedTheme) {
   ASSERT_TRUE(base::DeleteFile(theme_file, false));  // Not recursive.
 }
 
-// Tests that we can change the ID of an unpacked extension by adding a key
-// to its manifest.
-TEST_F(ExtensionServiceTest, UnpackedExtensionCanChangeID) {
-  InitializeEmptyExtensionService();
-
-  base::ScopedTempDir temp;
-  ASSERT_TRUE(temp.CreateUniqueTempDir());
-
-  base::FilePath extension_path = temp.path();
-  base::FilePath manifest_path =
-      extension_path.Append(extensions::kManifestFilename);
-  base::FilePath manifest_no_key =
-      data_dir().AppendASCII("unpacked").AppendASCII("manifest_no_key.json");
-
-  base::FilePath manifest_with_key =
-      data_dir().AppendASCII("unpacked").AppendASCII("manifest_with_key.json");
-
-  ASSERT_TRUE(base::PathExists(manifest_no_key));
-  ASSERT_TRUE(base::PathExists(manifest_with_key));
-
-  // Load the unpacked extension with no key.
-  base::CopyFile(manifest_no_key, manifest_path);
-  extensions::UnpackedInstaller::Create(service())->Load(extension_path);
-
-  base::RunLoop().RunUntilIdle();
-  EXPECT_EQ(0u, GetErrors().size());
-  ASSERT_EQ(1u, loaded_.size());
-  EXPECT_EQ(1u, registry()->enabled_extensions().size());
-
-  // Add the key to the manifest.
-  base::CopyFile(manifest_with_key, manifest_path);
-  loaded_.clear();
-
-  // Reload the extensions.
-  service()->ReloadExtensionsForTest();
-  const Extension* extension = service()->GetExtensionById(unpacked, false);
-  EXPECT_EQ(unpacked, extension->id());
-  ASSERT_EQ(1u, loaded_.size());
-
-  // TODO(jstritar): Right now this just makes sure we don't crash and burn, but
-  // we should also test that preferences are preserved.
-}
-
 #if defined(OS_POSIX)
 TEST_F(ExtensionServiceTest, UnpackedExtensionMayContainSymlinkedFiles) {
   base::FilePath source_data_dir =
diff --git a/chrome/browser/extensions/installed_loader.cc b/chrome/browser/extensions/installed_loader.cc
index 55b3b78..1ca7718 100644
--- a/chrome/browser/extensions/installed_loader.cc
+++ b/chrome/browser/extensions/installed_loader.cc
@@ -169,6 +169,12 @@ InstalledLoader::~InstalledLoader() {
 }
 
 void InstalledLoader::Load(const ExtensionInfo& info, bool write_to_prefs) {
+  // TODO(asargent): add a test to confirm that we can't load extensions if
+  // their ID in preferences does not match the extension's actual ID.
+  if (invalid_extensions_.find(info.extension_path) !=
+      invalid_extensions_.end())
+    return;
+
   std::string error;
   scoped_refptr<const Extension> extension(NULL);
   if (info.extension_manifest) {
@@ -269,7 +275,8 @@ void InstalledLoader::LoadAllExtensions() {
                                    GetCreationFlags(info),
                                    &error));
 
-      if (!extension.get()) {
+      if (!extension.get() || extension->id() != info->extension_id) {
+        invalid_extensions_.insert(info->extension_path);
         ExtensionErrorReporter::GetInstance()->ReportLoadError(
             info->extension_path,
             error,
diff --git a/chrome/browser/extensions/installed_loader.h b/chrome/browser/extensions/installed_loader.h
index 8d8ec7f..ed50404 100644
--- a/chrome/browser/extensions/installed_loader.h
+++ b/chrome/browser/extensions/installed_loader.h
@@ -5,6 +5,10 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_INSTALLED_LOADER_H_
 #define CHROME_BROWSER_EXTENSIONS_INSTALLED_LOADER_H_
 
+#include <set>
+
+#include "base/files/file_path.h"
+
 class ExtensionService;
 
 namespace extensions {
@@ -37,6 +41,9 @@ class InstalledLoader {
   ExtensionRegistry* extension_registry_;
 
   ExtensionPrefs* extension_prefs_;
+
+  // Paths to invalid extension manifests, which should not be loaded.
+  std::set<base::FilePath> invalid_extensions_;
 };
 
 }  // namespace extensions