Browse Source

fix: `chrome.tabs` 'url' and 'title' are privileged information (#39609)

fix: tabs url and title are privileged information

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
trop[bot] 1 year ago
parent
commit
f818ec3295

+ 22 - 9
shell/browser/extensions/api/tabs/tabs_api.cc

@@ -301,6 +301,7 @@ ExtensionFunction::ResponseAction TabsQueryFunction::Run() {
 
     tabs::Tab tab;
     tab.id = contents->ID();
+    tab.title = base::UTF16ToUTF8(wc->GetTitle());
     tab.url = wc->GetLastCommittedURL().spec();
     tab.active = contents->IsFocused();
     tab.audible = contents->IsCurrentlyAudible();
@@ -322,12 +323,18 @@ ExtensionFunction::ResponseAction TabsGetFunction::Run() {
     return RespondNow(Error("No such tab"));
 
   tabs::Tab tab;
-
   tab.id = tab_id;
-  // TODO(nornagon): in Chrome, the tab URL is only available to extensions
-  // that have the "tabs" (or "activeTab") permission. We should do the same
-  // permission check here.
-  tab.url = contents->web_contents()->GetLastCommittedURL().spec();
+
+  // "title" and "url" properties are considered privileged data and can
+  // only be checked if the extension has the "tabs" permission or it has
+  // access to the WebContents's origin.
+  auto* wc = contents->web_contents();
+  if (extension()->permissions_data()->HasAPIPermissionForTab(
+          contents->ID(), mojom::APIPermissionID::kTab) ||
+      extension()->permissions_data()->HasHostPermission(wc->GetURL())) {
+    tab.url = wc->GetLastCommittedURL().spec();
+    tab.title = base::UTF16ToUTF8(wc->GetTitle());
+  }
 
   tab.active = contents->IsFocused();
 
@@ -609,10 +616,16 @@ ExtensionFunction::ResponseValue TabsUpdateFunction::GetResult() {
   auto* api_web_contents = electron::api::WebContents::From(web_contents_);
   tab.id = (api_web_contents ? api_web_contents->ID() : -1);
 
-  // TODO(nornagon): in Chrome, the tab URL is only available to extensions
-  // that have the "tabs" (or "activeTab") permission. We should do the same
-  // permission check here.
-  tab.url = web_contents_->GetLastCommittedURL().spec();
+  // "title" and "url" properties are considered privileged data and can
+  // only be checked if the extension has the "tabs" permission or it has
+  // access to the WebContents's origin.
+  if (extension()->permissions_data()->HasAPIPermissionForTab(
+          api_web_contents->ID(), mojom::APIPermissionID::kTab) ||
+      extension()->permissions_data()->HasHostPermission(
+          web_contents_->GetURL())) {
+    tab.url = web_contents_->GetLastCommittedURL().spec();
+    tab.title = base::UTF16ToUTF8(web_contents_->GetTitle());
+  }
 
   if (api_web_contents)
     tab.active = api_web_contents->IsFocused();

+ 6 - 0
shell/common/extensions/api/_permission_features.json

@@ -20,5 +20,11 @@
     "extension_types": [
       "extension"
     ]
+  },
+  "tabs": {
+    "channel": "stable",
+    "extension_types": [
+      "extension"
+    ]
   }
 }

+ 2 - 0
shell/common/extensions/electron_extensions_api_provider.cc

@@ -38,6 +38,8 @@ constexpr APIPermissionInfo::InitInfo permissions_to_register[] = {
     {mojom::APIPermissionID::kPdfViewerPrivate, "pdfViewerPrivate"},
 #endif
     {mojom::APIPermissionID::kManagement, "management"},
+    {mojom::APIPermissionID::kTab, "tabs",
+     APIPermissionInfo::kFlagRequiresManagementUIWarning},
 };
 base::span<const APIPermissionInfo::InitInfo> GetPermissionInfos() {
   return base::make_span(permissions_to_register);

+ 61 - 21
spec/extensions-spec.ts

@@ -810,15 +810,14 @@ describe('chrome extensions', () => {
 
       before(async () => {
         customSession = session.fromPartition(`persist:${uuid.v4()}`);
-        await customSession.loadExtension(path.join(fixtures, 'extensions', 'tabs-api-async'));
+        await customSession.loadExtension(path.join(fixtures, 'extensions', 'chrome-tabs', 'api-async'));
       });
 
       beforeEach(() => {
         w = new BrowserWindow({
           show: false,
           webPreferences: {
-            session: customSession,
-            nodeIntegration: true
+            session: customSession
           }
         });
       });
@@ -881,27 +880,55 @@ describe('chrome extensions', () => {
         });
       });
 
-      it('get', async () => {
-        await w.loadURL(url);
+      describe('get', () => {
+        it('returns tab properties', async () => {
+          await w.loadURL(url);
 
-        const message = { method: 'get' };
-        w.webContents.executeJavaScript(`window.postMessage('${JSON.stringify(message)}', '*')`);
+          const message = { method: 'get' };
+          w.webContents.executeJavaScript(`window.postMessage('${JSON.stringify(message)}', '*')`);
 
-        const [,, responseString] = await once(w.webContents, 'console-message');
+          const [,, responseString] = await once(w.webContents, 'console-message');
 
-        const response = JSON.parse(responseString);
-        expect(response).to.have.property('active').that.is.a('boolean');
-        expect(response).to.have.property('autoDiscardable').that.is.a('boolean');
-        expect(response).to.have.property('discarded').that.is.a('boolean');
-        expect(response).to.have.property('groupId').that.is.a('number');
-        expect(response).to.have.property('highlighted').that.is.a('boolean');
-        expect(response).to.have.property('id').that.is.a('number');
-        expect(response).to.have.property('incognito').that.is.a('boolean');
-        expect(response).to.have.property('index').that.is.a('number');
-        expect(response).to.have.property('pinned').that.is.a('boolean');
-        expect(response).to.have.property('selected').that.is.a('boolean');
-        expect(response).to.have.property('url').that.is.a('string');
-        expect(response).to.have.property('windowId').that.is.a('number');
+          const response = JSON.parse(responseString);
+          expect(response).to.have.property('url').that.is.a('string');
+          expect(response).to.have.property('title').that.is.a('string');
+          expect(response).to.have.property('active').that.is.a('boolean');
+          expect(response).to.have.property('autoDiscardable').that.is.a('boolean');
+          expect(response).to.have.property('discarded').that.is.a('boolean');
+          expect(response).to.have.property('groupId').that.is.a('number');
+          expect(response).to.have.property('highlighted').that.is.a('boolean');
+          expect(response).to.have.property('id').that.is.a('number');
+          expect(response).to.have.property('incognito').that.is.a('boolean');
+          expect(response).to.have.property('index').that.is.a('number');
+          expect(response).to.have.property('pinned').that.is.a('boolean');
+          expect(response).to.have.property('selected').that.is.a('boolean');
+          expect(response).to.have.property('windowId').that.is.a('number');
+        });
+
+        it('does not return privileged properties without tabs permission', async () => {
+          const noPrivilegeSes = session.fromPartition(`persist:${uuid.v4()}`);
+          await noPrivilegeSes.loadExtension(path.join(fixtures, 'extensions', 'chrome-tabs', 'no-privileges'));
+
+          w = new BrowserWindow({ show: false, webPreferences: { session: noPrivilegeSes } });
+          await w.loadURL(url);
+
+          w.webContents.executeJavaScript('window.postMessage(\'{}\', \'*\')');
+          const [,, responseString] = await once(w.webContents, 'console-message');
+          const response = JSON.parse(responseString);
+          expect(response).not.to.have.property('url');
+          expect(response).not.to.have.property('title');
+          expect(response).to.have.property('active').that.is.a('boolean');
+          expect(response).to.have.property('autoDiscardable').that.is.a('boolean');
+          expect(response).to.have.property('discarded').that.is.a('boolean');
+          expect(response).to.have.property('groupId').that.is.a('number');
+          expect(response).to.have.property('highlighted').that.is.a('boolean');
+          expect(response).to.have.property('id').that.is.a('number');
+          expect(response).to.have.property('incognito').that.is.a('boolean');
+          expect(response).to.have.property('index').that.is.a('number');
+          expect(response).to.have.property('pinned').that.is.a('boolean');
+          expect(response).to.have.property('selected').that.is.a('boolean');
+          expect(response).to.have.property('windowId').that.is.a('number');
+        });
       });
 
       it('reload', async () => {
@@ -928,6 +955,19 @@ describe('chrome extensions', () => {
         const [,, responseString] = await once(w.webContents, 'console-message');
         const response = JSON.parse(responseString);
 
+        expect(response).to.have.property('url').that.is.a('string');
+        expect(response).to.have.property('title').that.is.a('string');
+        expect(response).to.have.property('active').that.is.a('boolean');
+        expect(response).to.have.property('autoDiscardable').that.is.a('boolean');
+        expect(response).to.have.property('discarded').that.is.a('boolean');
+        expect(response).to.have.property('groupId').that.is.a('number');
+        expect(response).to.have.property('highlighted').that.is.a('boolean');
+        expect(response).to.have.property('id').that.is.a('number');
+        expect(response).to.have.property('incognito').that.is.a('boolean');
+        expect(response).to.have.property('index').that.is.a('number');
+        expect(response).to.have.property('pinned').that.is.a('boolean');
+        expect(response).to.have.property('selected').that.is.a('boolean');
+        expect(response).to.have.property('windowId').that.is.a('number');
         expect(response).to.have.property('mutedInfo').that.is.a('object');
         const { mutedInfo } = response;
         expect(mutedInfo).to.deep.eq({

+ 0 - 0
spec/fixtures/extensions/tabs-api-async/background.js → spec/fixtures/extensions/chrome-tabs/api-async/background.js


+ 0 - 0
spec/fixtures/extensions/tabs-api-async/main.js → spec/fixtures/extensions/chrome-tabs/api-async/main.js


+ 2 - 1
spec/fixtures/extensions/tabs-api-async/manifest.json → spec/fixtures/extensions/chrome-tabs/api-async/manifest.json

@@ -1,5 +1,5 @@
 {
-  "name": "tabs-api-async",
+  "name": "api-async",
   "version": "1.0",
   "content_scripts": [
     {
@@ -8,6 +8,7 @@
       "run_at": "document_start"
     }
   ],
+  "permissions": ["tabs"],
   "background": {
     "service_worker": "background.js"
   },

+ 6 - 0
spec/fixtures/extensions/chrome-tabs/no-privileges/background.js

@@ -0,0 +1,6 @@
+/* global chrome */
+
+chrome.runtime.onMessage.addListener((_request, sender, sendResponse) => {
+  chrome.tabs.get(sender.tab.id).then(sendResponse);
+  return true;
+});

+ 11 - 0
spec/fixtures/extensions/chrome-tabs/no-privileges/main.js

@@ -0,0 +1,11 @@
+/* global chrome */
+
+chrome.runtime.onMessage.addListener((request, sender, sendResponse) => {
+  sendResponse(request);
+});
+
+window.addEventListener('message', () => {
+  chrome.runtime.sendMessage({}, response => {
+    console.log(JSON.stringify(response));
+  });
+}, false);

+ 19 - 0
spec/fixtures/extensions/chrome-tabs/no-privileges/manifest.json

@@ -0,0 +1,19 @@
+{
+  "name": "no-privileges",
+  "version": "1.0",
+  "content_scripts": [
+    {
+      "matches": [
+        "<all_urls>"
+      ],
+      "js": [
+        "main.js"
+      ],
+      "run_at": "document_start"
+    }
+  ],
+  "background": {
+    "service_worker": "background.js"
+  },
+  "manifest_version": 3
+}