Skip to content

Instantly share code, notes, and snippets.

@fmarier
Created July 8, 2022 23:31
Show Gist options
  • Save fmarier/3cf69f2d6696d44919784b62d9613a06 to your computer and use it in GitHub Desktop.
Save fmarier/3cf69f2d6696d44919784b62d9613a06 to your computer and use it in GitHub Desktop.
Patch for testing extension blocklisting in Chromium (https://bugs.chromium.org/p/chromium/issues/detail?id=1217487)
commit 8cad760b6aa583ab576fc7e69a5dab6091bef4ab
Author: Francois Marier <francois@brave.com>
Date: Mon Jun 7 20:56:21 2021 -0700
Add a --safebrowsing-manual-extension-blocklist flag.
This lets users add one or more extension IDs (separated by commas)
that should be marked as "BLOCKLISTED_MALWARE" as if they were
malware. This is used for testing the crx-list-info endpoint.
BUG=1217487
diff --git a/chrome/browser/extensions/BUILD.gn b/chrome/browser/extensions/BUILD.gn
index 73237cdd03390..ac2bfda8cd6aa 100644
--- a/chrome/browser/extensions/BUILD.gn
+++ b/chrome/browser/extensions/BUILD.gn
@@ -752,6 +752,7 @@ static_library("extensions") {
"//chrome/common/extensions/api",
"//components/omnibox/browser",
"//components/safe_browsing/core/browser/db:util",
+ "//components/safe_browsing/core/common",
"//components/safe_browsing/core/common/proto:csd_proto",
"//components/signin/core/browser",
"//components/translate/content/browser",
diff --git a/chrome/browser/extensions/blocklist.cc b/chrome/browser/extensions/blocklist.cc
index 36c301c185381..a293700c9a1ca 100644
--- a/chrome/browser/extensions/blocklist.cc
+++ b/chrome/browser/extensions/blocklist.cc
@@ -10,20 +10,25 @@
#include "base/bind.h"
#include "base/callback_list.h"
+#include "base/command_line.h"
#include "base/containers/contains.h"
#include "base/lazy_instance.h"
#include "base/memory/ref_counted.h"
#include "base/observer_list.h"
+#include "base/single_thread_task_runner.h"
+#include "base/strings/string_split.h"
#include "base/task/single_thread_task_runner.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/extensions/blocklist_factory.h"
#include "chrome/browser/extensions/blocklist_state_fetcher.h"
#include "chrome/browser/safe_browsing/safe_browsing_service.h"
+#include "components/crx_file/id_util.h"
#include "components/prefs/pref_service.h"
#include "components/safe_browsing/buildflags.h"
#include "components/safe_browsing/core/browser/db/database_manager.h"
#include "components/safe_browsing/core/browser/db/util.h"
+#include "components/safe_browsing/core/common/safebrowsing_switches.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
#include "extensions/browser/extension_prefs.h"
@@ -35,6 +40,9 @@ namespace extensions {
namespace {
+// This extension is marked as BLOCKLISTED_MALWARE in ChromeExtMalware.store.
+constexpr char kBlocklistedExtensionId[] = "ppjlgejclaebfbcijblljfengkoodjdl";
+
// The safe browsing database manager to use. Make this a global/static variable
// rather than a member of Blocklist because Blocklist accesses the real
// database manager before it has a chance to get a fake one.
@@ -86,6 +94,7 @@ class SafeBrowsingClientImpl
// Constructs a client to query the database manager for |extension_ids| and
// run |callback| with the IDs of those which have been blocklisted.
static void Start(const std::set<std::string>& extension_ids,
+ const std::set<std::string>& manual_blocklist_ids,
OnResultCallback callback) {
auto safe_browsing_client = base::WrapRefCounted(
new SafeBrowsingClientImpl(extension_ids, std::move(callback)));
@@ -93,7 +102,7 @@ class SafeBrowsingClientImpl
FROM_HERE,
base::BindOnce(&SafeBrowsingClientImpl::StartCheck,
safe_browsing_client, g_database_manager.Get().get(),
- extension_ids));
+ extension_ids, manual_blocklist_ids));
}
private:
@@ -109,9 +118,23 @@ class SafeBrowsingClientImpl
// Pass |database_manager| as a parameter to avoid touching
// SafeBrowsingService on the IO thread.
void StartCheck(scoped_refptr<SafeBrowsingDatabaseManager> database_manager,
- const std::set<std::string>& extension_ids) {
+ const std::set<std::string>& extension_ids,
+ const std::set<std::string>& manual_blocklist_ids) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- if (database_manager->CheckExtensionIDs(extension_ids, this)) {
+ DCHECK(extension_ids_.empty());
+ DCHECK(replaced_ids_.empty());
+
+ // Replaces manually-blocklisted IDs with a known-bad extension ID.
+ for (const std::string& id : extension_ids) {
+ if (manual_blocklist_ids.count(id) > 0) {
+ extension_ids_.insert(kBlocklistedExtensionId);
+ replaced_ids_.insert(id);
+ } else {
+ extension_ids_.insert(id);
+ }
+ }
+
+ if (database_manager->CheckExtensionIDs(extension_ids_, this)) {
// Definitely not blocklisted. Callback immediately.
callback_task_runner_->PostTask(
FROM_HERE,
@@ -125,13 +148,26 @@ class SafeBrowsingClientImpl
void OnCheckExtensionsResult(const std::set<std::string>& hits) override {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
- callback_task_runner_->PostTask(FROM_HERE,
- base::BindOnce(std::move(callback_), hits));
+
+ // Replaces the known-bad ID with the original extension IDs.
+ std::set<std::string> real_hits;
+ for (const std::string& id : hits) {
+ if (id == kBlocklistedExtensionId) {
+ real_hits.insert(replaced_ids_.cbegin(), replaced_ids_.cend());
+ } else {
+ real_hits.insert(id);
+ }
+ }
+
+ callback_task_runner_->PostTask(
+ FROM_HERE, base::BindOnce(std::move(callback_), real_hits));
Release(); // Balanced in StartCheck.
}
scoped_refptr<base::SingleThreadTaskRunner> callback_task_runner_;
OnResultCallback callback_;
+ std::set<std::string> extension_ids_;
+ std::set<std::string> replaced_ids_;
};
void CheckOneExtensionState(Blocklist::IsBlocklistedCallback callback,
@@ -174,6 +210,7 @@ Blocklist::Blocklist(ExtensionPrefs* prefs) {
lazy_database_manager.RegisterDatabaseChangedCallback(base::BindRepeating(
&Blocklist::ObserveNewDatabase, base::Unretained(this)));
+ ParseManualBlocklistFlag();
ObserveNewDatabase();
}
@@ -199,8 +236,9 @@ void Blocklist::GetBlocklistedIDs(const std::set<std::string>& ids,
// extensions returned by SafeBrowsing will then be passed to
// GetBlocklistStateIDs to get the particular BlocklistState for each id.
SafeBrowsingClientImpl::Start(
- ids, base::BindOnce(&Blocklist::GetBlocklistStateForIDs, AsWeakPtr(),
- std::move(callback)));
+ ids, manual_blocklist_ids_,
+ base::BindOnce(&Blocklist::GetBlocklistStateForIDs, AsWeakPtr(),
+ std::move(callback)));
}
void Blocklist::GetMalwareIDs(const std::set<std::string>& ids,
@@ -209,6 +247,30 @@ void Blocklist::GetMalwareIDs(const std::set<std::string>& ids,
std::move(callback)));
}
+void Blocklist::ParseManualBlocklistFlag() {
+ base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+ if (!command_line->HasSwitch(
+ safe_browsing::switches::kSbManualExtensionBlocklist))
+ return;
+
+ std::string flag_val = command_line->GetSwitchValueASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist);
+ for (const std::string& extension_id : base::SplitString(
+ flag_val, ",", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY)) {
+ if (crx_file::id_util::IdIsValid(extension_id)) {
+ manual_blocklist_ids_.insert(extension_id);
+ }
+ }
+}
+
+bool Blocklist::HasManualBlocklistIdsForTest() const {
+ return !manual_blocklist_ids_.empty();
+}
+
+bool Blocklist::IsIdManuallyBlocklisted(const std::string& extension_id) const {
+ return manual_blocklist_ids_.count(extension_id) > 0;
+}
+
void Blocklist::IsBlocklisted(const std::string& extension_id,
IsBlocklistedCallback callback) {
std::set<std::string> check;
@@ -276,7 +338,7 @@ void Blocklist::RequestExtensionsBlocklistState(
std::move(callback));
for (const auto& id : ids) {
state_fetcher_->Request(
- id,
+ IsIdManuallyBlocklisted(id) ? kBlocklistedExtensionId : id,
base::BindOnce(&Blocklist::OnBlocklistStateReceived, AsWeakPtr(), id));
}
}
diff --git a/chrome/browser/extensions/blocklist.h b/chrome/browser/extensions/blocklist.h
index 707bf262d7bd7..83179d281b20e 100644
--- a/chrome/browser/extensions/blocklist.h
+++ b/chrome/browser/extensions/blocklist.h
@@ -116,6 +116,10 @@ class Blocklist : public KeyedService, public base::SupportsWeakPtr<Blocklist> {
// whether the database is ready.
void IsDatabaseReady(DatabaseReadyCallback callback);
+ // Returns true if any extension IDs have been blocklisted on the
+ // command-line.
+ bool HasManualBlocklistIdsForTest() const;
+
private:
friend class ScopedDatabaseManagerForTest;
@@ -141,6 +145,13 @@ class Blocklist : public KeyedService, public base::SupportsWeakPtr<Blocklist> {
void ReturnBlocklistStateMap(GetBlocklistedIDsCallback callback,
const std::set<std::string>& blocklisted_ids);
+ // Parses a flag of blocklisted extension IDs to treat as malware.
+ // This is used for testing.
+ void ParseManualBlocklistFlag();
+
+ // Return true if this extension ID is blocklisted via flag (for testing).
+ bool IsIdManuallyBlocklisted(const std::string& extension_id) const;
+
base::ObserverList<Observer>::Unchecked observers_;
base::CallbackListSubscription database_updated_subscription_;
@@ -160,6 +171,10 @@ class Blocklist : public KeyedService, public base::SupportsWeakPtr<Blocklist> {
// is a pair of [vector of string ids to check, response closure].
std::list<std::pair<std::vector<std::string>, base::OnceClosure>>
state_requests_;
+
+ // Empty unless the --safebrowsing-manual-extension-blocklist commandline
+ // flag is used.
+ std::set<std::string> manual_blocklist_ids_;
};
} // namespace extensions
diff --git a/chrome/browser/extensions/blocklist_unittest.cc b/chrome/browser/extensions/blocklist_unittest.cc
index b078f932837dd..5d3bf8bbbf1a4 100644
--- a/chrome/browser/extensions/blocklist_unittest.cc
+++ b/chrome/browser/extensions/blocklist_unittest.cc
@@ -5,7 +5,9 @@
#include "chrome/browser/extensions/blocklist.h"
#include "base/bind.h"
+#include "base/command_line.h"
#include "base/run_loop.h"
+#include "base/test/scoped_command_line.h"
#include "base/threading/thread_task_runner_handle.h"
#include "chrome/browser/extensions/blocklist_state_fetcher.h"
#include "chrome/browser/extensions/fake_safe_browsing_database_manager.h"
@@ -13,6 +15,7 @@
#include "chrome/browser/extensions/test_blocklist.h"
#include "chrome/browser/extensions/test_blocklist_state_fetcher.h"
#include "chrome/browser/extensions/test_extension_prefs.h"
+#include "components/safe_browsing/core/common/safebrowsing_switches.h"
#include "content/public/test/browser_task_environment.h"
#include "extensions/browser/extension_prefs.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -190,4 +193,66 @@ TEST_F(BlocklistTest, FetchBlocklistStates) {
EXPECT_EQ(0U, cached_states.count(c));
}
+TEST_F(BlocklistTest, HasManualBlocklistIds_Default) {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist, "");
+
+ Blocklist blocklist(prefs());
+ EXPECT_FALSE(blocklist.HasManualBlocklistIdsForTest());
+}
+
+TEST_F(BlocklistTest, HasManualBlocklistIds_InvalidId) {
+ {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist, "invalid-id");
+
+ Blocklist blocklist(prefs());
+ EXPECT_FALSE(blocklist.HasManualBlocklistIdsForTest());
+ }
+
+ {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist,
+ "invalid-id,not-valid-either,,");
+
+ Blocklist blocklist(prefs());
+ EXPECT_FALSE(blocklist.HasManualBlocklistIdsForTest());
+ }
+}
+
+TEST_F(BlocklistTest, HasManualBlocklistIds_ValidId) {
+ {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist,
+ "gmbmikajjgmnabiglmofipeabaddhgne");
+
+ Blocklist blocklist(prefs());
+ EXPECT_TRUE(blocklist.HasManualBlocklistIdsForTest());
+ }
+
+ {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist,
+ "gmbmikajjgmnabiglmofipeabaddhgne,aapbdbdomjkkjkaonfhkkikfgjllcleb");
+
+ Blocklist blocklist(prefs());
+ EXPECT_TRUE(blocklist.HasManualBlocklistIdsForTest());
+ }
+
+ {
+ base::test::ScopedCommandLine scoped_command_line;
+ base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+ safe_browsing::switches::kSbManualExtensionBlocklist,
+ "invalid-id,,gmbmikajjgmnabiglmofipeabaddhgne");
+
+ Blocklist blocklist(prefs());
+ EXPECT_TRUE(blocklist.HasManualBlocklistIdsForTest());
+ }
+}
+
} // namespace extensions
diff --git a/components/safe_browsing/core/common/safebrowsing_switches.cc b/components/safe_browsing/core/common/safebrowsing_switches.cc
index 61fec0807e996..449d2d81b3017 100644
--- a/components/safe_browsing/core/common/safebrowsing_switches.cc
+++ b/components/safe_browsing/core/common/safebrowsing_switches.cc
@@ -16,6 +16,12 @@ namespace switches {
const char kSbManualDownloadBlocklist[] =
"safebrowsing-manual-download-blacklist";
+// List of comma-separated extension IDs which should be treated as
+// BLOCKLISTED_MALWARE. This is used for manual testing when looking
+// for ways to trigger the crx-list-info request.
+const char kSbManualExtensionBlocklist[] =
+ "safebrowsing-manual-extension-blocklist";
+
// Enable Safe Browsing Enhanced Protection.
const char kSbEnableEnhancedProtection[] =
"safebrowsing-enable-enhanced-protection";
diff --git a/components/safe_browsing/core/common/safebrowsing_switches.h b/components/safe_browsing/core/common/safebrowsing_switches.h
index 85ab6299acf37..aa93c268b7476 100644
--- a/components/safe_browsing/core/common/safebrowsing_switches.h
+++ b/components/safe_browsing/core/common/safebrowsing_switches.h
@@ -9,6 +9,7 @@ namespace safe_browsing {
namespace switches {
extern const char kSbManualDownloadBlocklist[];
+extern const char kSbManualExtensionBlocklist[];
extern const char kSbEnableEnhancedProtection[];
} // namespace switches
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment