Do not allow wildcard content settings entries for "shields enabled"

components/brave_shields/browser/brave_shields_util.cc

@@ -7,7 +7,9 @@
 
 #include <memory>
 
+#include "base/debug/dump_without_crashing.h"
 #include "base/feature_list.h"
+#include "base/logging.h"
 #include "base/notreached.h"
 #include "base/strings/string_number_conversions.h"
 #include "brave/components/brave_shields/browser/brave_shields_p3a.h"
@@ -181,12 +183,30 @@ void SetBraveShieldsEnabled(HostContentSettingsMap* map,
   if (url.is_valid() && !url.SchemeIsHTTPOrHTTPS())
     return;
 
-  DCHECK(!url.is_empty()) << "url for shields setting cannot be blank";
+  if (url.is_empty()) {
+    LOG(ERROR) << "url for shields setting cannot be blank";
+    return;
+  }
 
   auto primary_pattern = GetPatternFromURL(url);
 
-  if (!primary_pattern.IsValid())
+  if (primary_pattern.MatchesAllHosts()) {
+    LOG(ERROR) << "Url for shields setting cannot be blank or result in a "
+                  "wildcard content setting.";
+
+#if DCHECK_IS_ON()
+    DCHECK(false);
+#else
+    base::debug::DumpWithoutCrashing();
+#endif
     return;
+  }
+
+  if (!primary_pattern.IsValid()) {
+    DLOG(ERROR) << "Invalid primary pattern for Url: "
+                << url.possibly_invalid_spec();
+    return;
+  }
 
   map->SetContentSettingCustomScope(
       primary_pattern, ContentSettingsPattern::Wildcard(),

components/content_settings/core/browser/brave_content_settings_pref_provider.cc

@@ -157,6 +157,10 @@ void BravePrefProvider::MigrateShieldsSettings(bool incognito) {
                 *shields_cookies);
   }
 
+  // Fix any wildcard entries that could cause issues like
+  // https://github.com/brave/brave-browser/issues/23113
+  EnsureNoWildcardEntries();
+
   // Prior to Chromium 88, we used the "plugins" ContentSettingsType along with
   // ResourceIdentifiers to store our settings, which we need to migrate now
   // first of all, before attempting any other migration.
@@ -168,6 +172,17 @@ void BravePrefProvider::MigrateShieldsSettings(bool incognito) {
   MigrateShieldsSettingsV2ToV3();
 }
 
+void BravePrefProvider::EnsureNoWildcardEntries() {
+  // ContentSettingsType::BRAVE_SHIELDS should not have wildcard entries, i.e.
+  // there is no global disabled value.
+  // TODO(petemill): This should also be done for the other shields
+  // content settings types, and we can use default boolean prefs to represent
+  // defaults, e.g. `profile.default_content_setting_values.https_everywhere`.
+  SetWebsiteSetting(ContentSettingsPattern::Wildcard(),
+                    ContentSettingsPattern::Wildcard(),
+                    ContentSettingsType::BRAVE_SHIELDS, base::Value(), {});
+}
+
 void BravePrefProvider::MigrateShieldsSettingsFromResourceIds() {
   BravePrefProvider::CopyPluginSettingsForMigration(prefs_);
 
@@ -552,8 +567,12 @@ void BravePrefProvider::UpdateCookieRules(ContentSettingsType content_type,
 
   // Adding shields down rules (they always override cookie rules).
   for (const auto& shield_rule : shield_rules) {
-    // There is no global shields rule
-    DCHECK(!shield_rule.primary_pattern.MatchesAllHosts());
+    // There is no global shields rule, so if we have one ignore it. It would
+    // get replaced with EnsureNoWildcardEntries().
+    if (shield_rule.primary_pattern.MatchesAllHosts()) {
+      LOG(ERROR) << "Found a wildcard shields rule which matches all hosts.";
+      continue;
+    }
 
     // Shields down.
     if (ValueToContentSetting(shield_rule.value) == CONTENT_SETTING_BLOCK) {

components/content_settings/core/browser/brave_content_settings_pref_provider.h

@@ -58,7 +58,9 @@ class BravePrefProvider : public PrefProvider,
                            TestShieldsSettingsMigrationFromResourceIDs);
   FRIEND_TEST_ALL_PREFIXES(BravePrefProviderTest,
                            TestShieldsSettingsMigrationFromUnknownSettings);
+  FRIEND_TEST_ALL_PREFIXES(BravePrefProviderTest, EnsureNoWildcardEntries);
   void MigrateShieldsSettings(bool incognito);
+  void EnsureNoWildcardEntries();
   void MigrateShieldsSettingsFromResourceIds();
   void MigrateShieldsSettingsFromResourceIdsForOneType(
       const std::string& preference_path,

components/content_settings/core/browser/brave_content_settings_pref_provider_unittest.cc

@@ -13,6 +13,7 @@
 #include "brave/components/content_settings/core/browser/brave_content_settings_pref_provider.h"
 #include "brave/components/content_settings/core/browser/brave_content_settings_utils.h"
 #include "chrome/test/base/testing_profile.h"
+#include "components/content_settings/core/browser/content_settings_pref.h"
 #include "components/content_settings/core/browser/content_settings_registry.h"
 #include "components/content_settings/core/common/content_settings.h"
 #include "components/content_settings/core/common/content_settings_pattern.h"
@@ -611,4 +612,25 @@ TEST_F(BravePrefProviderTest, TestShieldsSettingsMigrationV2toV3) {
   provider.ShutdownOnUIThread();
 }
 
+TEST_F(BravePrefProviderTest, EnsureNoWildcardEntries) {
+  BravePrefProvider provider(
+      testing_profile()->GetPrefs(), false /* incognito */,
+      true /* store_last_modified */, false /* restore_session */);
+  ShieldsEnabledSetting shields_enabled_settings(&provider);
+  GURL example_url("https://example.com");
+  shields_enabled_settings.CheckSettingsAreDefault(example_url);
+  // Set wildcard entry
+  auto pattern = ContentSettingsPattern::Wildcard();
+  provider.SetWebsiteSetting(pattern, pattern,
+                             ContentSettingsType::BRAVE_SHIELDS,
+                             base::Value(CONTENT_SETTING_ALLOW), {});
+  // Verify global has changed
+  shields_enabled_settings.CheckSettingsWouldAllow(example_url);
+  // Remove wildcards
+  provider.EnsureNoWildcardEntries();
+  // Verify global has reset
+  shields_enabled_settings.CheckSettingsAreDefault(example_url);
+  provider.ShutdownOnUIThread();
+}
+
 }  //  namespace content_settings