[Merge m101][Extensions] Fix inadvertent leakage of info through even…

…t listeners.

Different extension event implementations (e.g. tabs.onCreated) can
tweak their listener params through Event::WillDispatchCallback
callback. It does so by passing mutable Event* to
WillDispatchCallback. This can lead to one extension's
event listener inadvertently affecting a different extension's
event listener, as the (modified) `Event*` is carried over to
next/subsequent listeners.

This CL changes this by removing the mutable Event param from
WillDispatchCallback, and exposes  "writable" `event_args_out`
and `event_filtering_info_out` params to WillDispatchCallback. This
is done so that interested parties can set those modified params
to be used by dispatcher (EventRouter::DispatchExtensionMessage).

Note that events funneled through LazyEventDispatcher is not
currently susceptible to this problem as it copies Event-s before
dispatching them.

(cherry picked from commit 940ddcf)

Bug: 1302959
Change-Id: I07c2bfd68b62412f93dcb5fd46315e3b4b496ddd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3531082
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#985564}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3584544
Reviewed-by: David Bertoni <dbertoni@chromium.org>
Cr-Commit-Position: refs/branch-heads/4951@{#707}
Cr-Branched-From: 27de622-refs/heads/main@{#982481}

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

@@ -916,8 +916,9 @@ bool OnDeterminingFilenameWillDispatchCallback(
     content::BrowserContext* browser_context,
     Feature::Context target_context,
     const Extension* extension,
-    Event* event,
-    const base::DictionaryValue* listener_filter) {
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   *any_determiners = true;
   base::Time installed =
       ExtensionPrefs::Get(browser_context)->GetInstallTime(extension->id());

chrome/browser/extensions/api/tabs/tabs_apitest.cc

@@ -17,9 +17,13 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/common/content_features.h"
 #include "content/public/test/browser_test.h"
+#include "content/public/test/browser_test_utils.h"
+#include "extensions/test/result_catcher.h"
+#include "extensions/test/test_extension_dir.h"
 #include "net/dns/mock_host_resolver.h"
 
 #if BUILDFLAG(IS_WIN)
@@ -379,6 +383,67 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTabTest, SendMessage) {
   ASSERT_TRUE(RunExtensionTest("tabs/send_message"));
 }
 
+// Tests that extension with "tabs" permission does not leak tab info to another
+// extension without "tabs" permission.
+//
+// Regression test for https://crbug.com/1302959
+IN_PROC_BROWSER_TEST_F(ExtensionApiTabTest, TabsPermissionDoesNotLeakTabInfo) {
+  constexpr char kManifestWithTabsPermission[] =
+      R"({
+        "name": "test", "version": "1", "manifest_version": 2,
+        "background": {"scripts": ["background.js"]},
+        "permissions": ["tabs"]
+      })";
+  constexpr char kBackgroundJSWithTabsPermission[] =
+      "chrome.tabs.onUpdated.addListener(() => {});";
+
+  constexpr char kManifestWithoutTabsPermission[] =
+      R"({
+        "name": "test", "version": "1", "manifest_version": 2,
+        "background": {"scripts": ["background.js"]}
+      })";
+  constexpr char kBackgroundJSWithoutTabsPermission[] =
+      R"(
+        let urlStr = '%s';
+        chrome.tabs.onUpdated.addListener(function(tabId, changeInfo, tab) {
+          chrome.test.assertEq(3, Array.from(arguments).length);
+          // Note: we'll search within all of the arguments, just to make sure
+          // we don't miss any inadvertently added ones. See
+          // https://crbug.com/1302959 for details.
+          let argumentsStr = JSON.stringify(arguments);
+          let containsUrlStr = argumentsStr.indexOf(urlStr) != -1;
+          chrome.test.assertFalse(containsUrlStr);
+          if (tab.status == 'complete') {
+            chrome.test.notifyPass();
+          }
+        });
+      )";
+
+  GURL url = embedded_test_server()->GetURL("/title1.html");
+
+  // First load the extension with "tabs" permission.
+  // Note that order is important for this regression test.
+  extensions::TestExtensionDir ext_dir1;
+  ext_dir1.WriteManifest(kManifestWithTabsPermission);
+  ext_dir1.WriteFile(FILE_PATH_LITERAL("background.js"),
+                     kBackgroundJSWithTabsPermission);
+  ASSERT_TRUE(LoadExtension(ext_dir1.UnpackedPath()));
+
+  // Then load the extension without "tabs" permission.
+  extensions::ResultCatcher catcher;
+  extensions::TestExtensionDir ext_dir2;
+  ext_dir2.WriteManifest(kManifestWithoutTabsPermission);
+  ext_dir2.WriteFile(FILE_PATH_LITERAL("background.js"),
+                     base::StringPrintf(kBackgroundJSWithoutTabsPermission,
+                                        url.spec().c_str()));
+  ASSERT_TRUE(LoadExtension(ext_dir2.UnpackedPath()));
+
+  // Now open a tab and ensure the extension in |ext_dir2| does not see any info
+  // that is guarded by "tabs" permission.
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), url));
+  EXPECT_TRUE(catcher.GetNextResult()) << catcher.message();
+}
+
 class IncognitoExtensionApiTabTest : public ExtensionApiTabTest,
                                      public testing::WithParamInterface<bool> {
 };

chrome/browser/extensions/api/tabs/tabs_event_router.cc

@@ -47,8 +47,9 @@ bool WillDispatchTabUpdatedEvent(
     content::BrowserContext* browser_context,
     Feature::Context target_context,
     const Extension* extension,
-    Event* event,
-    const base::DictionaryValue* listener_filter) {
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   ExtensionTabUtil::ScrubTabBehavior scrub_tab_behavior =
       ExtensionTabUtil::GetScrubTabBehavior(extension, target_context,
                                             contents);
@@ -65,19 +66,22 @@ bool WillDispatchTabUpdatedEvent(
       changed_properties.Set(property, value->Clone());
   }
 
-  event->event_args->Append(base::Value(std::move(changed_properties)));
-  event->event_args->Append(std::move(tab_value));
+  *event_args_out = std::make_unique<base::Value::List>();
+  (*event_args_out)->Append(ExtensionTabUtil::GetTabId(contents));
+  (*event_args_out)->Append(base::Value(std::move(changed_properties)));
+  (*event_args_out)->Append(std::move(tab_value));
   return true;
 }
 
-bool WillDispatchTabCreatedEvent(WebContents* contents,
-                                 bool active,
-                                 content::BrowserContext* browser_context,
-                                 Feature::Context target_context,
-                                 const Extension* extension,
-                                 Event* event,
-                                 const base::DictionaryValue* listener_filter) {
-  event->event_args->ClearList();
+bool WillDispatchTabCreatedEvent(
+    WebContents* contents,
+    bool active,
+    content::BrowserContext* browser_context,
+    Feature::Context target_context,
+    const Extension* extension,
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   ExtensionTabUtil::ScrubTabBehavior scrub_tab_behavior =
       ExtensionTabUtil::GetScrubTabBehavior(extension, target_context,
                                             contents);
@@ -86,7 +90,9 @@ bool WillDispatchTabCreatedEvent(WebContents* contents,
           ->ToValue());
   tab_value.SetBoolKey(tabs_constants::kSelectedKey, active);
   tab_value.SetBoolKey(tabs_constants::kActiveKey, active);
-  event->event_args->Append(std::move(tab_value));
+
+  *event_args_out = std::make_unique<base::Value::List>();
+  (*event_args_out)->Append(std::move(tab_value));
   return true;
 }
 
@@ -585,24 +591,13 @@ void TabsEventRouter::DispatchTabUpdatedEvent(
   DCHECK(!changed_property_names.empty());
   DCHECK(contents);
 
-  // The state of the tab (as seen from the extension point of view) has
-  // changed.  Send a notification to the extension.
-  std::unique_ptr<base::ListValue> args_base(new base::ListValue);
-
-  // First arg: The id of the tab that changed.
-  args_base->Append(ExtensionTabUtil::GetTabId(contents));
-
-  // Second arg: An object containing the changes to the tab state.  Filled in
-  // by WillDispatchTabUpdatedEvent as a copy of changed_properties, if the
-  // extension has the tabs permission.
-
-  // Third arg: An object containing the state of the tab. Filled in by
-  // WillDispatchTabUpdatedEvent.
   Profile* profile = Profile::FromBrowserContext(contents->GetBrowserContext());
 
   auto event = std::make_unique<Event>(
       events::TABS_ON_UPDATED, api::tabs::OnUpdated::kEventName,
-      std::move(*args_base).TakeListDeprecated(), profile);
+      // The event arguments depend on the extension's permission. They are set
+      // in WillDispatchTabUpdatedEvent().
+      std::vector<base::Value>(), profile);
   event->user_gesture = EventRouter::USER_GESTURE_NOT_ENABLED;
   event->will_dispatch_callback =
       base::BindRepeating(&WillDispatchTabUpdatedEvent, contents,

chrome/browser/extensions/api/tabs/windows_event_router.cc

@@ -59,12 +59,14 @@ bool ControllerVisibleToListener(WindowController* window_controller,
              WindowController::GetFilterFromWindowTypesValues(filter_value));
 }
 
-bool WillDispatchWindowEvent(WindowController* window_controller,
-                             BrowserContext* browser_context,
-                             Feature::Context target_context,
-                             const Extension* extension,
-                             Event* event,
-                             const base::DictionaryValue* listener_filter) {
+bool WillDispatchWindowEvent(
+    WindowController* window_controller,
+    BrowserContext* browser_context,
+    Feature::Context target_context,
+    const Extension* extension,
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   bool has_filter =
       listener_filter &&
       listener_filter->FindKey(extensions::tabs_constants::kWindowTypesKey);
@@ -75,15 +77,15 @@ bool WillDispatchWindowEvent(WindowController* window_controller,
     return false;
   }
 
-  // Cleanup previous values.
-  event->filter_info = mojom::EventFilteringInfo::New();
+  *event_filtering_info_out = mojom::EventFilteringInfo::New();
   // Only set the window type if the listener has set a filter.
   // Otherwise we set the window visibility relative to the extension.
   if (has_filter) {
-    event->filter_info->window_type = window_controller->GetWindowTypeText();
+    (*event_filtering_info_out)->window_type =
+        window_controller->GetWindowTypeText();
   } else {
-    event->filter_info->has_window_exposed_by_default = true;
-    event->filter_info->window_exposed_by_default = true;
+    (*event_filtering_info_out)->has_window_exposed_by_default = true;
+    (*event_filtering_info_out)->window_exposed_by_default = true;
   }
   return true;
 }
@@ -93,8 +95,9 @@ bool WillDispatchWindowFocusedEvent(
     BrowserContext* browser_context,
     Feature::Context target_context,
     const Extension* extension,
-    Event* event,
-    const base::DictionaryValue* listener_filter) {
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   int window_id = extension_misc::kUnknownWindowId;
   Profile* new_active_context = nullptr;
   bool has_filter =
@@ -108,19 +111,18 @@ bool WillDispatchWindowFocusedEvent(
     new_active_context = window_controller->profile();
   }
 
-  // Cleanup previous values.
-  event->filter_info = mojom::EventFilteringInfo::New();
+  *event_filtering_info_out = mojom::EventFilteringInfo::New();
   // Only set the window type if the listener has set a filter,
   // otherwise set the visibility to true (if the window is not
   // supposed to be visible by the extension, we will clear out the
   // window id later).
   if (has_filter) {
-    event->filter_info->window_type =
+    (*event_filtering_info_out)->window_type =
         window_controller ? window_controller->GetWindowTypeText()
                           : extensions::tabs_constants::kWindowTypeValueNormal;
   } else {
-    event->filter_info->has_window_exposed_by_default = true;
-    event->filter_info->window_exposed_by_default = true;
+    (*event_filtering_info_out)->has_window_exposed_by_default = true;
+    (*event_filtering_info_out)->window_exposed_by_default = true;
   }
 
   // When switching between windows in the default and incognito profiles,
@@ -135,12 +137,11 @@ bool WillDispatchWindowFocusedEvent(
   bool visible_to_listener = ControllerVisibleToListener(
       window_controller, extension, listener_filter);
 
+  *event_args_out = std::make_unique<base::Value::List>();
   if (cant_cross_incognito || !visible_to_listener) {
-    event->event_args->ClearList();
-    event->event_args->Append(extension_misc::kUnknownWindowId);
+    (*event_args_out)->Append(extension_misc::kUnknownWindowId);
   } else {
-    event->event_args->ClearList();
-    event->event_args->Append(window_id);
+    (*event_args_out)->Append(window_id);
   }
   return true;
 }

extensions/browser/api/audio/audio_api.cc

@@ -48,11 +48,13 @@ bool CanUseDeprecatedAudioApi(const Extension* extension) {
       .is_available();
 }
 
-bool CanReceiveDeprecatedAudioEvent(content::BrowserContext* browser_context,
-                                    Feature::Context target_context,
-                                    const Extension* extension,
-                                    Event* event,
-                                    const base::DictionaryValue* filter) {
+bool CanReceiveDeprecatedAudioEvent(
+    content::BrowserContext* browser_context,
+    Feature::Context target_context,
+    const Extension* extension,
+    const base::DictionaryValue* filter,
+    std::unique_ptr<base::Value::List>* event_args,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   return CanUseDeprecatedAudioApi(extension);
 }
 

extensions/browser/api/hid/hid_device_manager.cc

@@ -73,13 +73,15 @@ void PopulateHidDeviceInfo(hid::HidDeviceInfo* output,
   }
 }
 
-bool WillDispatchDeviceEvent(base::WeakPtr<HidDeviceManager> device_manager,
-                             const device::mojom::HidDeviceInfo& device_info,
-                             content::BrowserContext* browser_context,
-                             Feature::Context target_context,
-                             const Extension* extension,
-                             Event* event,
-                             const base::DictionaryValue* listener_filter) {
+bool WillDispatchDeviceEvent(
+    base::WeakPtr<HidDeviceManager> device_manager,
+    const device::mojom::HidDeviceInfo& device_info,
+    content::BrowserContext* browser_context,
+    Feature::Context target_context,
+    const Extension* extension,
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   if (device_manager && extension) {
     return device_manager->HasPermission(extension, device_info, false);
   }

extensions/browser/api/printer_provider/printer_provider_api.cc

@@ -297,12 +297,14 @@ class PrinterProviderAPIImpl : public PrinterProviderAPI,
   // in the event. If the extension listens to the event, it's added to the set
   // of |request| sources. |request| is |GetPrintersRequest| object associated
   // with the event.
-  bool WillRequestPrinters(int request_id,
-                           content::BrowserContext* browser_context,
-                           Feature::Context target_context,
-                           const Extension* extension,
-                           Event* event,
-                           const base::DictionaryValue* listener_filter);
+  bool WillRequestPrinters(
+      int request_id,
+      content::BrowserContext* browser_context,
+      Feature::Context target_context,
+      const Extension* extension,
+      const base::DictionaryValue* listener_filter,
+      std::unique_ptr<base::Value::List>* event_args_out,
+      mojom::EventFilteringInfoPtr* event_filtering_info_out);
 
   raw_ptr<content::BrowserContext> browser_context_;
 
@@ -759,8 +761,9 @@ bool PrinterProviderAPIImpl::WillRequestPrinters(
     content::BrowserContext* browser_context,
     Feature::Context target_context,
     const Extension* extension,
-    Event* event,
-    const base::DictionaryValue* listener_filter) {
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   if (!extension)
     return false;
   EventRouter* event_router = EventRouter::Get(browser_context_);

extensions/browser/api/usb/usb_device_manager.cc

@@ -64,12 +64,14 @@ bool ShouldExposeDevice(const device::mojom::UsbDeviceInfo& device_info) {
 
 // Returns true if the given extension has permission to receive events
 // regarding this device.
-bool WillDispatchDeviceEvent(const device::mojom::UsbDeviceInfo& device_info,
-                             content::BrowserContext* browser_context,
-                             Feature::Context target_context,
-                             const Extension* extension,
-                             Event* event,
-                             const base::DictionaryValue* listener_filter) {
+bool WillDispatchDeviceEvent(
+    const device::mojom::UsbDeviceInfo& device_info,
+    content::BrowserContext* browser_context,
+    Feature::Context target_context,
+    const Extension* extension,
+    const base::DictionaryValue* listener_filter,
+    std::unique_ptr<base::Value::List>* event_args_out,
+    mojom::EventFilteringInfoPtr* event_filtering_info_out) {
   // Check install-time and optional permissions.
   std::unique_ptr<UsbDevicePermission::CheckParam> param =
       UsbDevicePermission::CheckParam::ForUsbDevice(extension, device_info);