Do not crash while importing corrupted files.

Fixes brave/brave-browser#38332 Fixes brave/brave-browser#15895
brave · Oct 1, 2024 · 3764525 · 3764525
1 parent 9daf314
commit 3764525
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 4 deletions.
diff --git a/common/importer/chrome_importer_utils.cc b/common/importer/chrome_importer_utils.cc
@@ -43,8 +43,9 @@ std::optional<base::Value::Dict> GetChromeExtensionsListFromFile(
 
   std::optional<base::Value> preference =
       base::JSONReader::Read(preference_content);
-  DCHECK(preference);
-  DCHECK(preference->is_dict());
+  if (!preference || !preference->is_dict()) {
+    return std::nullopt;
+  }
   if (auto* extensions = preference->GetDict().FindDictByDottedPath(
           kChromeExtensionsListPath)) {
     return std::move(*extensions);
@@ -60,7 +61,9 @@ std::vector<std::string> GetImportableListFromChromeExtensionsList(
     const base::Value::Dict& extensions_list) {
   std::vector<std::string> extensions;
   for (const auto [key, value] : extensions_list) {
-    DCHECK(value.is_dict());
+    if (!value.is_dict()) {
+      continue;
+    }
     const base::Value::Dict& dict = value.GetDict();
     // Only import if type is extension, it's came from webstore and it's not
     // installed by default.
@@ -165,7 +168,9 @@ base::Value::List GetChromeSourceProfiles(
             continue;
 
           auto* name = profile->FindString("name");
-          DCHECK(name);
+          if (!name) {
+            continue;
+          }
           base::Value::Dict entry;
           entry.Set("id", value.first);
           entry.Set("name", *name);

diff --git a/common/importer/chrome_importer_utils_unittest.cc b/common/importer/chrome_importer_utils_unittest.cc
@@ -131,3 +131,21 @@ TEST_F(BraveChromeImporterUtilsTest, ChromeImporterCanImport) {
       ChromeImporterCanImport(GetTestProfilePath(), &services_supported));
   EXPECT_EQ(services_supported, importer::EXTENSIONS);
 }
+
+TEST_F(BraveChromeImporterUtilsTest, BadFiles) {
+  CopyTestFileToProfile("non_json_preferences",
+                        kChromeSecurePreferencesFile);
+  CopyTestFileToProfile("non_json_preferences", kChromePreferencesFile);
+  uint16_t services_supported = importer::NONE;
+  EXPECT_FALSE(
+      ChromeImporterCanImport(GetTestProfilePath(), &services_supported));
+  EXPECT_EQ(services_supported, importer::NONE);
+
+  CopyTestFileToProfile("non_dict_extension",
+                        kChromeSecurePreferencesFile);
+  CopyTestFileToProfile("non_dict_extension", kChromePreferencesFile);
+  services_supported = importer::NONE;
+  EXPECT_FALSE(
+      ChromeImporterCanImport(GetTestProfilePath(), &services_supported));
+  EXPECT_EQ(services_supported, importer::NONE);
+}
diff --git a/test/data/import/chrome/default/Local State b/test/data/import/chrome/default/Local State
@@ -12,6 +12,11 @@
             {
 
                 "name": "Profile 2"
+            },
+            "Profile 3":
+            {
+
+                "badname": "no crash please"
             }
         }
     }

diff --git a/test/data/import/chrome/default/non_dict_extension b/test/data/import/chrome/default/non_dict_extension
@@ -0,0 +1,7 @@
+{
+  "extensions" : {
+    "settings" : {
+      "someextensionid" : "not a dict for testing"
+    }
+  }
+}
diff --git a/test/data/import/chrome/default/non_json_preferences b/test/data/import/chrome/default/non_json_preferences
@@ -0,0 +1 @@
+abc