Bug 1985652: Use triggeringPrincipal rather than loadingPrincipal for CORS checks, if triggeringPrincipal is from an extension. a=RyanVM

This allows extensions to access their own content from scripts that they
injected into web pages, without that requested content being automatically
blocked due to failing CORS checks (since the Origin of the request is
the web page).

We have an existing pref 'extensions.content_web_accessible.enabled' whose
'false' value is meant to allow MV2 extensions to access their own resources
from their content-scripts (even if those resources aren't web-exposed). A
recent CORS strictness patch broke this use-case; so this patch relaxes the
strictness, specifically for extensions and specifically when that pref has
the permissive 'false' value.

While we're at it, this patch extends the related xpcshell test cover the case
where the pref is in its default/permissive 'false' configuration (to test the
regression being addressed by this patch), and to test that MV3 is strict under
that configuration. This patch also clarifies the other relevant pref for CORS
principal-selection to avoid implying that it's relevant to WebExtensions
(since it's not anymore).

Original Revision: https://phabricator.services.mozilla.com/D262863

Differential Revision: https://phabricator.services.mozilla.com/D263613
This commit is contained in:
Daniel Holbert
2025-09-02 11:54:50 +00:00
committed by rvandermeulen@mozilla.com
parent b75ffdc7ab
commit 59758e6b1d
3 changed files with 184 additions and 16 deletions

View File

@@ -348,6 +348,47 @@ static nsresult DoSOPChecks(nsIURI* aURI, nsILoadInfo* aLoadInfo,
return NS_OK;
}
// Determine which principal to use in DoCORSChecks. Normally, we do CORS
// checks using the LoadingPrincipal (whose Origin comes from the host
// document). But under certain configurations/situations, we instead use
// the TriggeringPrincipal() (whose Origin comes from the specific resource
// that initiated the request).
static nsIPrincipal* DeterminePrincipalForCORSChecks(nsILoadInfo* aLoadInfo) {
nsIPrincipal* const triggeringPrincipal = aLoadInfo->TriggeringPrincipal();
if (StaticPrefs::content_cors_use_triggering_principal()) {
// This pref forces us to use the TriggeringPrincipal.
// TODO(dholbert): Remove this special-case, perhaps right after we
// fix bug 1982916 which requires it for a test.
return triggeringPrincipal;
}
if (!StaticPrefs::extensions_content_web_accessible_enabled() &&
triggeringPrincipal->GetIsAddonOrExpandedAddonPrincipal()) {
// If we get here, then we know:
// * we want to allow MV2 WebExtensions to access their own resources
// regardless of whether those are listed in 'web_accessible_resources' in
// their manifest (this is nonstandard but it's a legacy thing we allow).
// * this load was initiated by a WebExtension (possibly running in a
// content script in the context of a web page).
//
// Hence: in this case, we use the TriggeringPrincipal for our CORS checks
// (so that a WebExtension requesting its own resources will be treated as
// same-origin, rather than being rejected as a cross-origin request from
// the page's origin).
//
// NOTE: Technically we should also check whether the extension uses MV2
// here, since this pref is specific to MV2. But that's not strictly
// necessary because we already unconditionally block MV3-WebExtension
// content-loads of this type at a different level (in
// nsScriptSecurityManager::CheckLoadURIWithPrincipal).
return triggeringPrincipal;
}
// Otherwise we use the LoadingPrincipal.
return aLoadInfo->GetLoadingPrincipal();
}
static nsresult DoCORSChecks(nsIChannel* aChannel, nsILoadInfo* aLoadInfo,
nsCOMPtr<nsIStreamListener>& aInAndOutListener) {
MOZ_RELEASE_ASSERT(aInAndOutListener,
@@ -358,19 +399,11 @@ static nsresult DoCORSChecks(nsIChannel* aChannel, nsILoadInfo* aLoadInfo,
return NS_OK;
}
nsIPrincipal* principal = aLoadInfo->GetLoadingPrincipal();
if (StaticPrefs::content_cors_use_triggering_principal()) {
// We use the triggering principal here, rather than the loading principal,
// to ensure that WebExtensions can reuse their own resources from content
// that they inject into a page.
//
// TODO(dholbert): Is there actually a legitimate reason that WebExtensions
// might need this (as opposed to exposing their resources for use in
// web-content via the 'web_accessible_resources' manifest field)?
principal = aLoadInfo->TriggeringPrincipal();
}
nsIPrincipal* principalForCORSCheck =
DeterminePrincipalForCORSChecks(aLoadInfo);
RefPtr<nsCORSListenerProxy> corsListener = new nsCORSListenerProxy(
aInAndOutListener, principal,
aInAndOutListener, principalForCORSCheck,
aLoadInfo->GetCookiePolicy() == nsILoadInfo::SEC_COOKIES_INCLUDE);
// XXX: @arg: DataURIHandling::Allow
// lets use DataURIHandling::Allow for now and then decide on callsite basis.

View File

@@ -2189,8 +2189,12 @@
mirror: always
# If true, we'll use the triggering principal rather than the loading principal
# when doing CORS checks. This might be needed for WebExtensions to load their
# own resources from content that they inject into sites.
# when doing CORS checks. Doing so is generally not correct, but we shipped
# with this behavior for a long time, so we're leaving it preffable in case
# it's needed for testing or if we discover some edge case that requires it.
#
# TODO(dholbert): Remove this pref after the 'false' default-value has
# successfully shipped for a release or two.
- name: content.cors.use_triggering_principal
type: bool
value: false

View File

@@ -1,5 +1,8 @@
"use strict";
const WEB_ACCESSIBLE_STRICTNESS_PREF =
"extensions.content_web_accessible.enabled";
const server = createHttpServer({ hosts: ["example.com"] });
server.registerPathHandler("/dummy", (request, response) => {
@@ -69,8 +72,56 @@ add_task(async function test_disallowed_import() {
await contentPage.close();
});
add_task(async function test_normal_import() {
Services.prefs.setBoolPref("extensions.content_web_accessible.enabled", true);
add_task(async function test_import_non_web_accessible_non_strict() {
// "non_strict" in the function name is referring to this pref. Setting it
// to 'false' should allow MV2 extensions to dynamically load content
// even if that content isn't in web_accessible_resources (and that's
// what the rest of this function tests).
Services.prefs.setBoolPref(WEB_ACCESSIBLE_STRICTNESS_PREF, false);
let extension = ExtensionTestUtils.loadExtension({
manifest: {
content_scripts: [
{
matches: ["http://example.com/dummy"],
js: ["main.js"],
},
],
},
files: {
"main.js": async function () {
try {
let mod = await import(browser.runtime.getURL("module1.js"));
browser.test.assertEq(mod.bar, 2);
browser.test.assertEq(mod.counter(), 0);
} catch (e) {
browser.test.assertTrue(false, `Threw while importing module: ${e}`);
}
browser.test.sendMessage("done");
},
"module1.js": MODULE1,
"module2.js": MODULE2,
},
});
await extension.startup();
let contentPage = await ExtensionTestUtils.loadContentPage(
"http://example.com/dummy"
);
await extension.awaitMessage("done");
await extension.unload();
await contentPage.close();
Services.prefs.clearUserPref(WEB_ACCESSIBLE_STRICTNESS_PREF);
});
add_task(async function test_import_non_web_accessible_strict() {
// "strict" in the function name is referring to this pref. Setting it
// to "true" should prevent MV2 extensions from dynamically loading content
// unless that content is listed in web_accessible_resources (and that's
// what the rest of this function tests).
Services.prefs.setBoolPref(WEB_ACCESSIBLE_STRICTNESS_PREF, true);
let extension = ExtensionTestUtils.loadExtension({
manifest: {
@@ -142,6 +193,86 @@ add_task(async function test_normal_import() {
await extension.unload();
await contentPage.close();
Services.prefs.clearUserPref(WEB_ACCESSIBLE_STRICTNESS_PREF);
});
add_task(async function test_import_non_web_accessible_mv3() {
// Here we set the prev to the non-strict value, but we still expect
// the same as the "strict" test above, because our extension is using
// 'manifest_version: 3' which should be strict regardless of the pref value.
Services.prefs.setBoolPref(WEB_ACCESSIBLE_STRICTNESS_PREF, false);
let extension = ExtensionTestUtils.loadExtension({
manifest: {
manifest_version: 3,
content_scripts: [
{
matches: ["http://example.com/dummy"],
js: ["main.js"],
},
],
},
files: {
"main.js": async function () {
const url = browser.runtime.getURL("module1.js");
await browser.test.assertRejects(
import(url),
/error loading dynamically imported module/,
"Cannot import script that is not web-accessible from page context"
);
await browser.test.assertRejects(
window.eval(`import("${url}")`),
/error loading dynamically imported module/,
"Cannot import script that is not web-accessible from page context"
);
let promise = new Promise((resolve, reject) => {
exportFunction(resolve, window, { defineAs: "resolve" });
exportFunction(reject, window, { defineAs: "reject" });
});
window.setTimeout(`import("${url}").then(resolve, reject)`, 0);
await browser.test.assertRejects(
promise,
/error loading dynamically imported module/,
"Cannot import script that is not web-accessible from page context"
);
browser.test.sendMessage("done");
},
"module1.js": MODULE1,
"module2.js": MODULE2,
},
});
await extension.startup();
let contentPage = await ExtensionTestUtils.loadContentPage(
"http://example.com/dummy"
);
await extension.awaitMessage("done");
// Web page can not import non-web-accessible files.
await contentPage.spawn([extension.uuid], async uuid => {
let files = ["main.js", "module1.js", "module2.js"];
for (let file of files) {
let url = `moz-extension://${uuid}/${file}`;
await Assert.rejects(
content.eval(`import("${url}")`),
/error loading dynamically imported module/,
"Cannot import script that is not web-accessible"
);
}
});
await extension.unload();
await contentPage.close();
Services.prefs.clearUserPref(WEB_ACCESSIBLE_STRICTNESS_PREF);
});
add_task(async function test_import_web_accessible() {