Closed Bug 1760451 Opened 2 years ago Closed 2 years ago

Enforce origin permissions in the new scripting api

Categories

(WebExtensions :: General, task, P2)

task

Tracking

(firefox102 fixed)

RESOLVED FIXED
102 Branch
Tracking Status
firefox102 --- fixed

People

(Reporter: zombie, Assigned: willdurand)

References

(Blocks 1 open bug)

Details

(Whiteboard: [addons-jira][design-decision-approved])

Attachments

(1 file)

This test passes without extension listing any host permission, even as optional:
https://searchfox.org/mozilla-central/rev/92ae17163d/toolkit/components/extensions/test/xpcshell/test_ext_scripting_contentScripts.js

Current implementation of scripting.registerContentScript (and related methods) doesn't check origin permissions, and allows extensions to register a content script that will run in any page, regardless of extensions permissions.

This was probably done under assumption that WebExtensionContentScript would enforce origin permissions, but that's not the case (at least not until bug 1745819).

WebExtensionContentScript didn't enforce origin permissions because we used content script match patterns from the manifest as permissions in the initial extension install prompt. That's why contentScripts.register checked against allowedOrigins.


We should audit the new scripting api for other issues where origin permissions should be enforced. At the minimum, registerContentScript (and related) should check if the script match pattern is subsumed by optional host permissions.

Furthermore, I think it should probably check against actually granted permissions, extensions are able to see which permissions they were granted, and prompt for any additional needed before calling this method. That would be the preferable UX, instead of depending on Firefox to inform/prompt the user to grant them.

Blocks: 1687764
Whiteboard: [addons-jira]
Depends on: 1745819

Going from Rob's suggestion, I will be adding a checkPermissions flag to WebExtensionContentScript in bug 1745819, so that this api could use it in mv2 as well.

Severity: -- → S2
Priority: -- → P2
Whiteboard: [addons-jira] → [addons-jira][design-decision-needed]

We have test coverage for the scripting API, as in that dynamically registered content scripts won't run if an optional permission is revoked, in https://searchfox.org/mozilla-central/rev/625c3d0c8ae46502aed83f33bd530cb93e926e9f/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_permissions_change.js.

It would be nice to also have a simple test that confirms that without any host permissions, that scripting.registerContentScripts aren't executed when a content page is loaded. For example by having the example.com permission, and opening example.net (no access) and example.com, and verifying that the content script executed (e.g. observed by a browser.test.sendMessage call from the content script). If example.net is loaded first, and example.com is loaded second, receiving a (first) message from example.com implies that the script didn't run at example.net, as desired.

Reclassifying this bug as a task now, because bug 1745819 fixes the lack of permission check, and AFAIK the only remaining task is writing a dedicated unit test.

I think that [design-decision-needed] was added to consider enabling this API (and enforcing the permission check) in MV2, but that could be a separate bug.

Severity: S2 → N/A
Type: defect → task

This is still a bug IMO, the content script currently only checks permissions before injecting in mv3, if we expose it in mv2 we will be opening a vulnerability.

Assignee: nobody → wdurand
Depends on: 1766615

I added some new tests for scripting.executeScript() but permissions seem enforced already.

Depends on D144803

Attachment #9274032 - Attachment description: Bug 1760451 - Check permissions for content scripts registered with the scripting APIs in MV2. r?robwu! → Bug 1760451 - Check permissions for content scripts registered with the scripting APIs. r?robwu!
See Also: → 1766822
Blocks: 1766852
Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4db3945f4fcb
Check permissions for content scripts registered with the scripting APIs. r=robwu
Whiteboard: [addons-jira][design-decision-needed] → [addons-jira][design-decision-approved]

Backed out for causing mochitest failures on test_ext_scripting_permissions.html.

Push with failures

Failure log

Backout link

[task 2022-05-06T13:19:03.285Z] 13:19:03     INFO - TEST-START | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html
[task 2022-05-06T13:19:03.509Z] 13:19:03     INFO - GECKO(4152) | Console message: Warning: attempting to write 10214 bytes to preference extensions.webextensions.uuids. This is bad for general performance and memory usage. Such an amount of data should rather be written to an external file. This preference will not be sent to any content processes.
[task 2022-05-06T13:19:05.202Z] 13:19:05     INFO - GECKO(4152) | Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.net/" line: 0}]
[task 2022-05-06T13:19:06.580Z] 13:19:06     INFO - GECKO(4152) | -----------------------------------------------------
[task 2022-05-06T13:19:06.582Z] 13:19:06     INFO - GECKO(4152) | Suppressions used:
[task 2022-05-06T13:19:06.583Z] 13:19:06     INFO - GECKO(4152) |   count      bytes template
[task 2022-05-06T13:19:06.585Z] 13:19:06     INFO - GECKO(4152) |      12        384 nsComponentManagerImpl
[task 2022-05-06T13:19:06.586Z] 13:19:06     INFO - GECKO(4152) |       2        288 libfontconfig.so
[task 2022-05-06T13:19:06.586Z] 13:19:06     INFO - GECKO(4152) | -----------------------------------------------------
[task 2022-05-06T13:19:06.895Z] 13:19:06     INFO - GECKO(4152) | Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.org/" line: 0}]
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) | -----------------------------------------------------
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) | Suppressions used:
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) |   count      bytes template
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) |      12        384 nsComponentManagerImpl
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) |       2        288 libfontconfig.so
[task 2022-05-06T13:19:06.980Z] 13:19:06     INFO - GECKO(4152) | -----------------------------------------------------
[task 2022-05-06T13:19:08.169Z] 13:19:08     INFO - GECKO(4152) | Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.com/" line: 0}]
[task 2022-05-06T13:19:08.797Z] 13:19:08     INFO - GECKO(4152) | Console message: [JavaScript Warning: "This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>”." {file: "https://example.org/?2" line: 0}]
[task 2022-05-06T13:19:51.012Z] 13:19:51     INFO - GECKO(4152) | Console message: [JavaScript Error: "Invalid ETag value "undefined"" {file: "resource://services-settings/SyncHistory.jsm" line: 50}]
[task 2022-05-06T13:19:51.014Z] 13:19:51     INFO - GECKO(4152) | store@resource://services-settings/SyncHistory.jsm:50:13
[task 2022-05-06T13:21:51.007Z] 13:21:51     INFO - GECKO(4152) | 1651843311006	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates
[task 2022-05-06T13:21:51.010Z] 13:21:51     INFO - GECKO(4152) | Console message: [JavaScript Error: "1651843311006	addons.xpi	ERROR	System addon update list error Error: got node name: html, expected: updates" {file: "resource://gre/modules/Log.jsm" line: 723}]
[task 2022-05-06T13:21:51.010Z] 13:21:51     INFO - GECKO(4152) | append@resource://gre/modules/Log.jsm:723:12
[task 2022-05-06T13:21:51.011Z] 13:21:51     INFO - GECKO(4152) | log@resource://gre/modules/Log.jsm:379:16
[task 2022-05-06T13:21:51.012Z] 13:21:51     INFO - GECKO(4152) | error@resource://gre/modules/Log.jsm:387:10
[task 2022-05-06T13:21:51.012Z] 13:21:51     INFO - GECKO(4152) | updateSystemAddons/res<@resource://gre/modules/addons/XPIInstall.jsm:4070:25
[task 2022-05-06T13:24:30.773Z] 13:24:30     INFO - TEST-INFO | started process screentopng
[task 2022-05-06T13:24:30.948Z] 13:24:30     INFO - TEST-INFO | screentopng: exit 0
[task 2022-05-06T13:24:30.948Z] 13:24:30     INFO - Buffered messages logged at 13:19:03
[task 2022-05-06T13:24:30.948Z] 13:24:30     INFO - add_task | Entering test setup
[task 2022-05-06T13:24:30.949Z] 13:24:30     INFO - add_task | Leaving test setup
[task 2022-05-06T13:24:30.950Z] 13:24:30     INFO - add_task | Entering test test_scripting_registerContentScripts_mv2
[task 2022-05-06T13:24:30.950Z] 13:24:30     INFO - Extension loaded
[task 2022-05-06T13:24:30.951Z] 13:24:30     INFO - Buffered messages logged at 13:19:08
[task 2022-05-06T13:24:30.952Z] 13:24:30     INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | expected: example.com, received: example.com 
[task 2022-05-06T13:24:30.952Z] 13:24:30     INFO - TEST-PASS | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | permission request succeeded 
[task 2022-05-06T13:24:30.953Z] 13:24:30     INFO - Buffered messages finished
[task 2022-05-06T13:24:30.954Z] 13:24:30     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | Test timed out. - 
[task 2022-05-06T13:24:31.790Z] 13:24:31     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-05-06T13:24:31.792Z] 13:24:31     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | Extension left running at test shutdown 
[task 2022-05-06T13:24:31.793Z] 13:24:31     INFO -     SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
[task 2022-05-06T13:24:31.793Z] 13:24:31     INFO -     ExtensionTestUtils.loadExtension/<@SimpleTest/ExtensionTestUtils.js:132:18
[task 2022-05-06T13:24:31.793Z] 13:24:31     INFO -     executeCleanupFunction@SimpleTest/SimpleTest.js:1487:13
[task 2022-05-06T13:24:31.794Z] 13:24:31     INFO -     SimpleTest.finish@SimpleTest/SimpleTest.js:1501:3
[task 2022-05-06T13:24:31.794Z] 13:24:31     INFO -     killTest@SimpleTest/TestRunner.js:194:22
[task 2022-05-06T13:24:31.805Z] 13:24:31     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-05-06T13:24:31.807Z] 13:24:31     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | no tasks awaiting on messages - got "[\"script-ran\"]", expected "[]"
[task 2022-05-06T13:24:31.808Z] 13:24:31     INFO -     SimpleTest.is@SimpleTest/SimpleTest.js:500:14
[task 2022-05-06T13:24:31.809Z] 13:24:31     INFO -     ExtensionTestUtils.loadExtension/<@SimpleTest/ExtensionTestUtils.js:51:18
[task 2022-05-06T13:24:31.809Z] 13:24:31     INFO -     executeCleanupFunction@SimpleTest/SimpleTest.js:1487:13
[task 2022-05-06T13:24:31.823Z] 13:24:31     INFO - Not taking screenshot here: see the one that was previously logged
[task 2022-05-06T13:24:31.824Z] 13:24:31     INFO - TEST-UNEXPECTED-FAIL | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | Test left extra windows or tabs: {"extraWindows":{},"extraTabs":{"0":"https://example.net/","1":"https://example.org/","2":"https://example.com/","3":"https://example.org/?2"}}
[task 2022-05-06T13:24:31.824Z] 13:24:31     INFO -  
[task 2022-05-06T13:24:31.825Z] 13:24:31     INFO -     SimpleTest.ok@SimpleTest/SimpleTest.js:417:16
[task 2022-05-06T13:24:31.826Z] 13:24:31     INFO -     @toolkit/components/extensions/test/mochitest/head.js:33:9
[task 2022-05-06T13:24:31.832Z] 13:24:31     INFO - GECKO(4152) | MEMORY STAT | vsize 20974622MB | residentFast 880MB
[task 2022-05-06T13:24:32.850Z] 13:24:32     INFO - TEST-OK | toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html | took 329566ms
[task 2022-05-06T13:24:35.855Z] 13:24:35     INFO - Error: Unable to restore focus, expect failures and timeouts.
[task 2022-05-06T13:24:35.873Z] 13:24:35    ERROR - TEST-UNEXPECTED-FAIL | /tests/toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html logged result after SimpleTest.finish(): Extension left running at test shutdown
[task 2022-05-06T13:24:35.875Z] 13:24:35    ERROR - TEST-UNEXPECTED-FAIL | /tests/toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html logged result after SimpleTest.finish(): no tasks awaiting on messages
[task 2022-05-06T13:24:35.877Z] 13:24:35    ERROR - TEST-UNEXPECTED-FAIL | /tests/toolkit/components/extensions/test/mochitest/test_ext_scripting_permissions.html logged result after SimpleTest.finish(): Test left extra windows or tabs: {"extraWindows":{},"extraTabs":{"0":"https://example.net/","1":"https://example.org/","2":"https://example.com/","3":"https://example.org/?2"}}
[task 2022-05-06T13:24:35.877Z] 13:24:35    ERROR - 
[task 2022-05-06T13:24:35.893Z] 13:24:35     INFO - TEST-START | toolkit/components/extensions/test/mochitest/test_ext_scripting_removeCSS.html
Flags: needinfo?(wdurand)
Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c7284bc0cb3a
Check permissions for content scripts registered with the scripting APIs. r=robwu

Backed out 2 changesets (Bug 1760451, Bug 1766615) for causing mochitest failures on test_ext_scripting_permissions.html.
Backout link
Push with failures <--> 2
Failure Log

Pushed by wdurand@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/3e8c75339427
Check permissions for content scripts registered with the scripting APIs. r=robwu
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 102 Branch
Flags: needinfo?(wdurand)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: