Enforce origin permissions in the new scripting api
Categories
(WebExtensions :: General, task, P2)
Tracking
(firefox102 fixed)
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.
Updated•2 years ago
|
Reporter | ||
Comment 1•2 years ago
|
||
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.
Comment 2•2 years ago
|
||
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.
Reporter | ||
Comment 3•2 years ago
•
|
||
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 | ||
Updated•2 years ago
|
Assignee | ||
Comment 4•2 years ago
|
||
I added some new tests for scripting.executeScript()
but permissions seem enforced already.
Depends on D144803
Updated•2 years ago
|
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
Updated•2 years ago
|
Comment 6•2 years ago
|
||
Backed out for causing mochitest failures on test_ext_scripting_permissions.html.
[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
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
Comment 8•2 years ago
|
||
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
Comment 10•2 years ago
|
||
bugherder |
Assignee | ||
Updated•2 years ago
|
Description
•