Don't let WebUI events cross the incognito barrier.

WebUI events use the same dispatch mechanism as extensions. Extensions
have a mechanism to allow some extension events to cross the incognito
barrier. These checks were inadvertently being skipped for WebUI and
webpages.

This CL adds the check for WebUI and webpages to enforce that no events
cross the incognito boundary. This CL updates SettingsPrivateEventRouter
to attach a browser context to the generated event.

Bug: 1381219
Change-Id: Iffa6fc40de737b353ab2705919f4863c583d2c5c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4786992
Reviewed-by: Emily Stark <estark@chromium.org>
Code-Coverage: findit-for-me@appspot.gserviceaccount.com <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Erik Chen <erikchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187516}

chrome/browser/chromeos/extensions/smart_card_provider_private/smart_card_provider_private_apitest.cc

@@ -294,7 +294,8 @@ class EventObserver : public EventRouter::TestObserver {
       run_loop_.Quit();
     }
   }
-  void OnDidDispatchEventToProcess(const Event& event) override {}
+  void OnDidDispatchEventToProcess(const Event& event,
+                                   int process_id) override {}
 
   std::map<std::string, size_t> event_count_;
   std::string expected_event_name_;

chrome/browser/extensions/api/downloads/downloads_api_browsertest.cc

@@ -232,7 +232,8 @@ class DownloadsEventsListener : public EventRouter::TestObserver {
   }
 
   // extensions::EventRouter::TestObserver:
-  void OnDidDispatchEventToProcess(const extensions::Event& event) override {}
+  void OnDidDispatchEventToProcess(const extensions::Event& event,
+                                   int process_id) override {}
 
   bool WaitFor(Profile* profile,
                const std::string& event_name,

chrome/browser/extensions/api/settings_private/settings_private_event_router.cc

@@ -161,9 +161,10 @@ void SettingsPrivateEventRouter::SendPrefChange(const std::string& pref_name) {
 
   auto args(api::settings_private::OnPrefsChanged::Create(prefs));
 
-  std::unique_ptr<Event> extension_event(new Event(
-      events::SETTINGS_PRIVATE_ON_PREFS_CHANGED,
-      api::settings_private::OnPrefsChanged::kEventName, std::move(args)));
+  std::unique_ptr<Event> extension_event =
+      std::make_unique<Event>(events::SETTINGS_PRIVATE_ON_PREFS_CHANGED,
+                              api::settings_private::OnPrefsChanged::kEventName,
+                              std::move(args), context_);
   event_router->BroadcastEvent(std::move(extension_event));
 }
 

chrome/browser/extensions/api/settings_private/settings_private_event_router_unittest.cc

@@ -0,0 +1,160 @@
+// Copyright 2023 The Chromium Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/extensions/api/settings_private/settings_private_event_router.h"
+
+#include "base/test/run_until.h"
+#include "chrome/browser/extensions/api/settings_private/settings_private_event_router_factory.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/test/base/testing_browser_process.h"
+#include "chrome/test/base/testing_profile.h"
+#include "chrome/test/base/testing_profile_manager.h"
+#include "content/public/test/browser_task_environment.h"
+#include "content/public/test/mock_render_process_host.h"
+#include "extensions/browser/event_router_factory.h"
+#include "extensions/browser/process_map.h"
+#include "extensions/browser/process_map_factory.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace extensions {
+namespace {
+
+// A fake that pretends that all contexts are WebUI.
+class ProcessMapFake : public ProcessMap {
+ public:
+  Feature::Context GetMostLikelyContextType(const Extension* extension,
+                                            int process_id,
+                                            const GURL* url) const override {
+    return Feature::WEBUI_CONTEXT;
+  }
+};
+
+std::unique_ptr<KeyedService> BuildEventRouter(
+    content::BrowserContext* profile) {
+  return std::make_unique<extensions::EventRouter>(profile, nullptr);
+}
+
+std::unique_ptr<KeyedService> BuildSettingsPrivateEventRouter(
+    content::BrowserContext* profile) {
+  return std::unique_ptr<KeyedService>(
+      SettingsPrivateEventRouter::Create(profile));
+}
+
+std::unique_ptr<KeyedService> BuildProcessMap(
+    content::BrowserContext* profile) {
+  return std::make_unique<ProcessMapFake>();
+}
+
+// Tracks event dispatches to a specific process.
+class EventRouterObserver : public EventRouter::TestObserver {
+ public:
+  // Only counts events that match |process_id|.
+  explicit EventRouterObserver(int process_id) : process_id_(process_id) {}
+
+  void OnWillDispatchEvent(const Event& event) override {
+    // Do nothing.
+  }
+
+  void OnDidDispatchEventToProcess(const Event& event,
+                                   int process_id) override {
+    if (process_id == process_id_) {
+      ++dispatch_count;
+    }
+  }
+
+  int dispatch_count = 0;
+  const int process_id_;
+};
+
+class SettingsPrivateEventRouterTest : public testing::Test {
+ public:
+  SettingsPrivateEventRouterTest()
+      : manager_(TestingBrowserProcess::GetGlobal()) {}
+  void SetUp() override { ASSERT_TRUE(manager_.SetUp()); }
+
+ protected:
+  content::BrowserTaskEnvironment task_environment_;
+  TestingProfileManager manager_;
+};
+
+// Tests that events from incognito profiles do not get routed to regular
+// profiles. Regression test for https://crbug.com/1381219.
+TEST_F(SettingsPrivateEventRouterTest, IncognitoEventRouting) {
+  // Create a testing profile. Override relevant factories.
+  TestingProfile* profile = manager_.CreateTestingProfile("test");
+  EventRouterFactory::GetInstance()->SetTestingFactory(
+      profile, base::BindRepeating(&BuildEventRouter));
+  SettingsPrivateEventRouterFactory::GetInstance()->SetTestingFactory(
+      profile, base::BindRepeating(&BuildSettingsPrivateEventRouter));
+  ProcessMapFactory::GetInstance()->SetTestingFactory(
+      profile, base::BindRepeating(&BuildProcessMap));
+
+  // Create an otr profile. Override relevant factories.
+  Profile::OTRProfileID otr_id = Profile::OTRProfileID::PrimaryID();
+  Profile* otr_profile =
+      profile->GetOffTheRecordProfile(otr_id, /*create_if_needed=*/true);
+  EventRouterFactory::GetInstance()->SetTestingFactory(
+      otr_profile, base::BindRepeating(&BuildEventRouter));
+  SettingsPrivateEventRouterFactory::GetInstance()->SetTestingFactory(
+      otr_profile, base::BindRepeating(&BuildSettingsPrivateEventRouter));
+  ProcessMapFactory::GetInstance()->SetTestingFactory(
+      otr_profile, base::BindRepeating(&BuildProcessMap));
+
+  // Create the event routers.
+  EventRouter* regular_event_router =
+      EventRouterFactory::GetInstance()->GetForBrowserContext(profile);
+  EventRouter* otr_event_router =
+      EventRouterFactory::GetInstance()->GetForBrowserContext(otr_profile);
+
+  // Today, EventRouter instances are shared between on- and off-the-record
+  // profile instances. We separate them into variables here, since the
+  // SettingsPrivateEventRouter shouldn't necessarily know about that or
+  // care.
+  EXPECT_EQ(regular_event_router, otr_event_router);
+
+  // Create the special routers for settingsPrivate.
+  ASSERT_TRUE(SettingsPrivateEventRouterFactory::GetForProfile(profile));
+  ASSERT_TRUE(SettingsPrivateEventRouterFactory::GetForProfile(otr_profile));
+
+  // Create some mock rphs.
+  content::MockRenderProcessHost regular_rph(profile);
+  content::MockRenderProcessHost otr_rph(otr_profile);
+
+  // Add event listeners, as if we had created two real WebUIs, one in a regular
+  // profile and one in an otr profile. Note that the string chrome://settings
+  // is hardcoded into the api permissions of settingsPrivate.
+  GURL kDummyURL("chrome://settings");
+  regular_event_router->AddEventListenerForURL(
+      api::settings_private::OnPrefsChanged::kEventName, &regular_rph,
+      kDummyURL);
+  otr_event_router->AddEventListenerForURL(
+      api::settings_private::OnPrefsChanged::kEventName, &otr_rph, kDummyURL);
+
+  // Hook up some test observers
+  EventRouterObserver regular_counter(regular_rph.GetID());
+  regular_event_router->AddObserverForTesting(&regular_counter);
+  EventRouterObserver otr_counter(otr_rph.GetID());
+  otr_event_router->AddObserverForTesting(&otr_counter);
+
+  EXPECT_EQ(0, regular_counter.dispatch_count);
+  EXPECT_EQ(0, otr_counter.dispatch_count);
+
+  // Setting an otr pref should not trigger the normal observer.
+  otr_profile->GetPrefs()->SetBoolean(prefs::kPromptForDownload, true);
+  ASSERT_TRUE(
+      base::test::RunUntil([&]() { return otr_counter.dispatch_count == 1; }));
+  EXPECT_EQ(0, regular_counter.dispatch_count);
+  EXPECT_EQ(1, otr_counter.dispatch_count);
+
+  // Setting a regular pref should not trigger the otr observer.
+  profile->GetPrefs()->SetBoolean(prefs::kPromptForDownload, true);
+  ASSERT_TRUE(base::test::RunUntil(
+      [&]() { return regular_counter.dispatch_count == 1; }));
+  EXPECT_EQ(1, regular_counter.dispatch_count);
+  EXPECT_EQ(1, otr_counter.dispatch_count);
+}
+
+}  // namespace
+}  // namespace extensions

chrome/browser/extensions/extension_service_test_base.h

@@ -121,7 +121,8 @@ class ExtensionServiceTestBase : public testing::Test {
   virtual void InitializeExtensionService(
       const ExtensionServiceInitParams& params);
 
-  // Initialize an empty ExtensionService using the default init params.
+  // Initialize an empty ExtensionService using a production, on-disk pref file.
+  // See documentation for |prefs_content|.
   void InitializeEmptyExtensionService();
 
   // Initialize an ExtensionService with a few already-installed extensions.

chrome/browser/ssl/ssl_browsertest.cc

@@ -2335,7 +2335,8 @@ IN_PROC_BROWSER_TEST_F(SSLUITest, TestExtensionEvents) {
       event_names_.push_back(event.event_name);
     }
 
-    void OnDidDispatchEventToProcess(const extensions::Event& event) override {}
+    void OnDidDispatchEventToProcess(const extensions::Event& event,
+                                     int process_id) override {}
 
     const std::vector<std::string>& event_names() const { return event_names_; }
 

chrome/test/BUILD.gn

@@ -8510,6 +8510,7 @@ test("unit_tests") {
       "../browser/extensions/api/search/search_api_unittest.cc",
       "../browser/extensions/api/settings_private/generated_pref_test_base.cc",
       "../browser/extensions/api/settings_private/generated_pref_test_base.h",
+      "../browser/extensions/api/settings_private/settings_private_event_router_unittest.cc",
       "../browser/extensions/api/socket/socket_api_unittest.cc",
       "../browser/extensions/api/socket/tcp_socket_unittest.cc",
       "../browser/extensions/api/socket/tls_socket_unittest.cc",

extensions/browser/api/web_request/permission_helper.cc

@@ -17,8 +17,10 @@ namespace extensions {
 
 PermissionHelper::PermissionHelper(content::BrowserContext* context)
     : browser_context_(context),
-      process_map_(ProcessMap::Get(context)),
-      extension_registry_(ExtensionRegistry::Get(context)) {}
+      extension_registry_(ExtensionRegistry::Get(context)) {
+  // Ensure the dependency is constructed.
+  ProcessMap::Get(browser_context_);
+}
 
 PermissionHelper::~PermissionHelper() = default;
 

extensions/browser/api/web_request/permission_helper.h

@@ -7,6 +7,7 @@
 
 #include "base/memory/raw_ptr.h"
 #include "extensions/browser/browser_context_keyed_api_factory.h"
+#include "extensions/browser/process_map.h"
 
 namespace extensions {
 class ExtensionRegistry;
@@ -33,7 +34,9 @@ class PermissionHelper : public BrowserContextKeyedAPI {
   bool ShouldHideBrowserNetworkRequest(const WebRequestInfo& request) const;
   bool CanCrossIncognito(const Extension* extension) const;
 
-  const ProcessMap* process_map() const { return process_map_; }
+  const ProcessMap* process_map() const {
+    return ProcessMap::Get(browser_context_);
+  }
 
   const ExtensionRegistry* extension_registry() const {
     return extension_registry_;
@@ -43,7 +46,6 @@ class PermissionHelper : public BrowserContextKeyedAPI {
   friend class BrowserContextKeyedAPIFactory<PermissionHelper>;
 
   const raw_ptr<content::BrowserContext> browser_context_;
-  const raw_ptr<ProcessMap> process_map_;
   const raw_ptr<ExtensionRegistry> extension_registry_;
 
   // BrowserContextKeyedAPI implementation.

extensions/browser/event_router.cc

@@ -146,6 +146,15 @@ LazyContextId LazyContextIdForListener(const EventListener* listener,
 // A global identifier used to distinguish extension events.
 base::AtomicSequenceNumber g_extension_event_id;
 
+// Returns whether an event would cross the incognito boundary. e.g.
+// incognito->regular or regular->incognito. This is allowed for some extensions
+// that enable spanning-mode but is always disallowed for webUI.
+// |context| refers to the BrowserContext of the receiver of the event.
+bool CrossesIncognito(BrowserContext* context, const Event& event) {
+  return event.restrict_to_browser_context &&
+         context != event.restrict_to_browser_context;
+}
+
 }  // namespace
 
 const char EventRouter::kRegisteredLazyEvents[] = "events";
@@ -256,8 +265,7 @@ bool EventRouter::CanDispatchEventToBrowserContext(BrowserContext* context,
                                                    const Event& event) {
   // Is this event from a different browser context than the renderer (ie, an
   // incognito tab event sent to a normal process, or vice versa).
-  bool crosses_incognito = event.restrict_to_browser_context &&
-                           context != event.restrict_to_browser_context;
+  bool crosses_incognito = CrossesIncognito(context, event);
   if (!crosses_incognito)
     return true;
   return ExtensionsBrowserClient::Get()->CanExtensionCrossIncognito(extension,
@@ -1036,6 +1044,12 @@ void EventRouter::DispatchEventToProcess(
     if (!CanDispatchEventToBrowserContext(listener_context, extension, event)) {
       return;
     }
+  } else {
+    // Non-extension (e.g. WebUI and web pages) checks. In general we don't
+    // allow context-bound events to cross the incognito barrier.
+    if (CrossesIncognito(listener_context, event)) {
+      return;
+    }
   }
 
   // TODO(ortuno): |listener_url| is passed in from the renderer so it can't
@@ -1106,8 +1120,9 @@ void EventRouter::DispatchEventToProcess(
                                                 worker_thread_id});
   }
 
-  for (TestObserver& observer : test_observers_)
-    observer.OnDidDispatchEventToProcess(event);
+  for (TestObserver& observer : test_observers_) {
+    observer.OnDidDispatchEventToProcess(event, process->GetID());
+  }
 
   // TODO(lazyboy): This is wrong for extensions SW events. We need to:
   // 1. Increment worker ref count

extensions/browser/event_router.h

@@ -111,7 +111,8 @@ class EventRouter : public KeyedService,
    public:
     virtual ~TestObserver() = default;
     virtual void OnWillDispatchEvent(const Event& event) = 0;
-    virtual void OnDidDispatchEventToProcess(const Event& event) = 0;
+    virtual void OnDidDispatchEventToProcess(const Event& event,
+                                             int process_id) = 0;
   };
 
   // Gets the EventRouter for |browser_context|.