From 183b0cfc30f2d40f818a08cbea960f6119e2d196 Mon Sep 17 00:00:00 2001 From: Alexandru Michis Date: Thu, 8 Jul 2021 22:56:34 +0300 Subject: [PATCH] Backed out changeset 19de2822bc0c (bug 1711168) for causing Bug 1719063. CLOSED TREE --- caps/BasePrincipal.cpp | 36 +-- caps/nsScriptSecurityManager.cpp | 50 ++--- dom/chrome-webidl/WebExtensionPolicy.webidl | 3 +- dom/security/nsContentSecurityManager.cpp | 5 +- netwerk/base/nsIProtocolHandler.idl | 9 +- .../protocol/res/ExtensionProtocolHandler.cpp | 13 +- toolkit/components/extensions/Schemas.jsm | 12 - .../extensions/WebExtensionPolicy.cpp | 14 +- .../extensions/WebExtensionPolicy.h | 11 +- .../extensions/schemas/manifest.json | 7 - ...st_ext_web_accessible_resources_matches.js | 209 +----------------- 11 files changed, 53 insertions(+), 316 deletions(-) diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index f9f205ef91b1..ccec8e35110d 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -18,7 +18,6 @@ #include "nsAboutProtocolUtils.h" #include "ThirdPartyUtil.h" #include "mozilla/ContentPrincipal.h" -#include "mozilla/ExtensionPolicyService.h" #include "mozilla/NullPrincipal.h" #include "mozilla/dom/BlobURLProtocolHandler.h" #include "mozilla/dom/ChromeUtils.h" @@ -571,32 +570,21 @@ nsresult BasePrincipal::CheckMayLoadHelper(nsIURI* aURI, } } - // Get the principal uri for the last flag check or error. - nsCOMPtr prinURI; - rv = GetURI(getter_AddRefs(prinURI)); - if (!(NS_SUCCEEDED(rv) && prinURI)) { - return NS_ERROR_DOM_BAD_URI; - } - - // If Extension uris are web accessible by this principal it is allowed to - // load. - bool maybeWebAccessible = false; - NS_URIChainHasFlags(aURI, nsIProtocolHandler::WEBEXT_URI_WEB_ACCESSIBLE, - &maybeWebAccessible); - NS_ENSURE_SUCCESS(rv, rv); - if (maybeWebAccessible) { - bool isWebAccessible = false; - rv = ExtensionPolicyService::GetSingleton().SourceMayLoadExtensionURI( - prinURI, aURI, &isWebAccessible); - if (NS_SUCCEEDED(rv) && isWebAccessible) { - return NS_OK; - } + bool fetchableByAnyone; + rv = NS_URIChainHasFlags(aURI, nsIProtocolHandler::URI_FETCHABLE_BY_ANYONE, + &fetchableByAnyone); + if (NS_SUCCEEDED(rv) && fetchableByAnyone) { + return NS_OK; } if (aReport) { - nsScriptSecurityManager::ReportError( - "CheckSameOriginError", prinURI, aURI, - mOriginAttributes.mPrivateBrowsingId > 0, aInnerWindowID); + nsCOMPtr prinURI; + rv = GetURI(getter_AddRefs(prinURI)); + if (NS_SUCCEEDED(rv) && prinURI) { + nsScriptSecurityManager::ReportError( + "CheckSameOriginError", prinURI, aURI, + mOriginAttributes.mPrivateBrowsingId > 0, aInnerWindowID); + } } return NS_ERROR_DOM_BAD_URI; diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp index ad866be78133..ef04bbe40c76 100644 --- a/caps/nsScriptSecurityManager.cpp +++ b/caps/nsScriptSecurityManager.cpp @@ -670,6 +670,21 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal, return NS_ERROR_DOM_BAD_URI; } + // Extensions may allow access to a web accessible resource. + bool maybeWebAccessible = false; + NS_URIChainHasFlags(targetBaseURI, + nsIProtocolHandler::WEBEXT_URI_WEB_ACCESSIBLE, + &maybeWebAccessible); + NS_ENSURE_SUCCESS(rv, rv); + if (maybeWebAccessible) { + bool isWebAccessible = false; + rv = ExtensionPolicyService::GetSingleton().SourceMayLoadExtensionURI( + sourceURI, targetBaseURI, &isWebAccessible); + if (!(NS_SUCCEEDED(rv) && isWebAccessible)) { + return NS_ERROR_DOM_BAD_URI; + } + } + // Check for uris that are only loadable by principals that subsume them bool targetURIIsLoadableBySubsumers = false; rv = NS_URIChainHasFlags(targetBaseURI, @@ -743,7 +758,6 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal, bool schemesMatch = scheme.Equals(otherScheme, nsCaseInsensitiveCStringComparator); bool isSamePage = false; - bool isExtensionMismatch = false; // about: URIs are special snowflakes. if (scheme.EqualsLiteral("about") && schemesMatch) { nsAutoCString moduleName, otherModuleName; @@ -791,13 +805,6 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal, } } } - } else if (schemesMatch && scheme.EqualsLiteral("moz-extension")) { - // If it is not the same exension, we want to ensure we end up - // calling CheckLoadURIFlags - nsAutoCString host, otherHost; - currentURI->GetHost(host); - currentOtherURI->GetHost(otherHost); - isExtensionMismatch = !host.Equals(otherHost); } else { bool equalExceptRef = false; rv = currentURI->EqualsExceptRef(currentOtherURI, &equalExceptRef); @@ -806,12 +813,10 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal, // If schemes are not equal, or they're equal but the target URI // is different from the source URI and doesn't always allow linking - // from the same scheme, or this is two different extensions, check - // if the URI flags of the current target URI allow the current - // source URI to link to it. + // from the same scheme, check if the URI flags of the current target + // URI allow the current source URI to link to it. // The policy is specified by the protocol flags on both URIs. - if (!schemesMatch || (denySameSchemeLinks && !isSamePage) || - isExtensionMismatch) { + if (!schemesMatch || (denySameSchemeLinks && !isSamePage)) { return CheckLoadURIFlags( currentURI, currentOtherURI, sourceBaseURI, targetBaseURI, aFlags, aPrincipal->OriginAttributesRef().mPrivateBrowsingId > 0, @@ -886,25 +891,6 @@ nsresult nsScriptSecurityManager::CheckLoadURIFlags( } } - // If Extension uris are web accessible they have WEBEXT_URI_WEB_ACCESSIBLE. - bool maybeWebAccessible = false; - NS_URIChainHasFlags(aTargetURI, nsIProtocolHandler::WEBEXT_URI_WEB_ACCESSIBLE, - &maybeWebAccessible); - NS_ENSURE_SUCCESS(rv, rv); - if (maybeWebAccessible) { - bool isWebAccessible = false; - rv = ExtensionPolicyService::GetSingleton().SourceMayLoadExtensionURI( - aSourceURI, aTargetURI, &isWebAccessible); - if (NS_SUCCEEDED(rv) && isWebAccessible) { - return NS_OK; - } - if (reportErrors) { - ReportError(errorTag, aSourceURI, aTargetURI, aFromPrivateWindow, - aInnerWindowID); - } - return NS_ERROR_DOM_BAD_URI; - } - // Check for chrome target URI bool targetURIIsUIResource = false; rv = NS_URIChainHasFlags(aTargetURI, nsIProtocolHandler::URI_IS_UI_RESOURCE, diff --git a/dom/chrome-webidl/WebExtensionPolicy.webidl b/dom/chrome-webidl/WebExtensionPolicy.webidl index 3263d7da7557..82d706421290 100644 --- a/dom/chrome-webidl/WebExtensionPolicy.webidl +++ b/dom/chrome-webidl/WebExtensionPolicy.webidl @@ -268,8 +268,7 @@ interface WebExtensionPolicy { dictionary WebAccessibleResourceInit { required sequence resources; - MatchPatternSetOrStringSequence? matches = null; - sequence? extensions = null; + MatchPatternSetOrStringSequence matches; }; dictionary WebExtensionInit { diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index bc6a6d4c8fdf..8acab09d2559 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -1080,7 +1080,7 @@ nsresult nsContentSecurityManager::CheckAllowLoadInSystemPrivilegedContext( } /* - * Every protocol handler must set one of the six security flags + * Every protocol handler must set one of the five security flags * defined in nsIProtocolHandler - if not - deny the load. */ nsresult nsContentSecurityManager::CheckChannelHasProtocolSecurityFlag( @@ -1105,9 +1105,6 @@ nsresult nsContentSecurityManager::CheckChannelHasProtocolSecurityFlag( NS_ENSURE_SUCCESS(rv, rv); uint32_t securityFlagsSet = 0; - if (flags & nsIProtocolHandler::WEBEXT_URI_WEB_ACCESSIBLE) { - securityFlagsSet += 1; - } if (flags & nsIProtocolHandler::URI_LOADABLE_BY_ANYONE) { securityFlagsSet += 1; } diff --git a/netwerk/base/nsIProtocolHandler.idl b/netwerk/base/nsIProtocolHandler.idl index 0d5c4d421341..15ac93cd916f 100644 --- a/netwerk/base/nsIProtocolHandler.idl +++ b/netwerk/base/nsIProtocolHandler.idl @@ -272,6 +272,13 @@ interface nsIProtocolHandler : nsISupports */ const unsigned long URI_IS_POTENTIALLY_TRUSTWORTHY = (1<<17); + /** + * This URI may be fetched and the contents are visible to anyone. This is + * semantically equivalent to the resource being served with all-access CORS + * headers. + */ + const unsigned long URI_FETCHABLE_BY_ANYONE = (1 << 18); + /** * If this flag is set, then the origin for this protocol is the full URI * spec, not just the scheme + host + port. @@ -305,7 +312,7 @@ interface nsIProtocolHandler : nsISupports /** * This is an extension web accessible uri that is loadable if checked - * against an allow whitelist using ExtensionPolicyService::SourceMayLoadExtensionURI. + * against an allow whitelist. */ const unsigned long WEBEXT_URI_WEB_ACCESSIBLE = (1 << 24); }; diff --git a/netwerk/protocol/res/ExtensionProtocolHandler.cpp b/netwerk/protocol/res/ExtensionProtocolHandler.cpp index aa4f610ff1a4..959e430d9c9a 100644 --- a/netwerk/protocol/res/ExtensionProtocolHandler.cpp +++ b/netwerk/protocol/res/ExtensionProtocolHandler.cpp @@ -368,12 +368,15 @@ nsresult ExtensionProtocolHandler::GetFlagsForURI(nsIURI* aURI, URLInfo url(aURI); if (auto* policy = EPS().GetByURL(url)) { - // In general a moz-extension URI is only loadable by chrome, but an - // allowlist subset are web-accessible (and cross-origin fetchable). - // The allowlist is checked using EPS.SourceMayLoadExtensionURI in - // BasePrincipal and nsScriptSecurityManager. + // In general a moz-extension URI is only loadable by chrome, but a + // whitelisted subset are web-accessible (and cross-origin fetchable). Check + // that whitelist. For Manifest V3 extensions, an additional whitelist + // for the source loading the url must be checked so we add the flag + // WEBEXT_URI_WEB_ACCESSIBLE, which is then checked in + // nsScriptSecurityManager. if (policy->IsWebAccessiblePath(url.FilePath())) { - flags |= WEBEXT_URI_WEB_ACCESSIBLE; + flags |= URI_LOADABLE_BY_ANYONE | URI_FETCHABLE_BY_ANYONE | + WEBEXT_URI_WEB_ACCESSIBLE; } else { flags |= URI_DANGEROUS_TO_LOAD; } diff --git a/toolkit/components/extensions/Schemas.jsm b/toolkit/components/extensions/Schemas.jsm index 48d6847bd36a..d90504572fbf 100644 --- a/toolkit/components/extensions/Schemas.jsm +++ b/toolkit/components/extensions/Schemas.jsm @@ -306,18 +306,6 @@ const POSTPROCESSORS = { context.logError(context.makeError(msg)); throw new Error(msg); }, - - webAcessibleMatching(value, context) { - // Ensure each object has at least one of matches or extensions array. - for (let obj of value) { - if (!obj.matches && !obj.extensions) { - const msg = `web_accessible_resources requires one of "matches" or "extensions"`; - context.logError(context.makeError(msg)); - throw new Error(msg); - } - } - return value; - }, }; // Parses a regular expression, with support for the Python extended diff --git a/toolkit/components/extensions/WebExtensionPolicy.cpp b/toolkit/components/extensions/WebExtensionPolicy.cpp index 896934158a00..dc3cec46613e 100644 --- a/toolkit/components/extensions/WebExtensionPolicy.cpp +++ b/toolkit/components/extensions/WebExtensionPolicy.cpp @@ -143,24 +143,12 @@ WebAccessibleResource::WebAccessibleResource( return; } - if (!aInit.mMatches.IsNull()) { + if (aInit.mMatches.WasPassed()) { MatchPatternOptions options; options.mRestrictSchemes = true; mMatches = ParseMatches(aGlobal, aInit.mMatches.Value(), options, ErrorBehavior::CreateEmptyPattern, aRv); } - - if (!aInit.mExtensions.IsNull()) { - mExtensions = new AtomSet(aInit.mExtensions.Value()); - } -} - -bool WebAccessibleResource::IsExtensionMatch(const URLInfo& aURI) { - if (!mExtensions) { - return false; - } - WebExtensionPolicy* policy = EPS().GetByHost(aURI.Host()); - return policy && mExtensions->Contains(policy->Id()); } NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(WebAccessibleResource) diff --git a/toolkit/components/extensions/WebExtensionPolicy.h b/toolkit/components/extensions/WebExtensionPolicy.h index 2f8fdcaab0db..3692fbdf662b 100644 --- a/toolkit/components/extensions/WebExtensionPolicy.h +++ b/toolkit/components/extensions/WebExtensionPolicy.h @@ -50,23 +50,16 @@ class WebAccessibleResource final : public nsISupports { } bool SourceMayAccessPath(const URLInfo& aURI, const nsAString& aPath) { - return mWebAccessiblePaths.Matches(aPath) && - (IsHostMatch(aURI) || IsExtensionMatch(aURI)); + return mWebAccessiblePaths.Matches(aPath) && mMatches && + mMatches->Matches(aURI); } - bool IsHostMatch(const URLInfo& aURI) { - return mMatches && mMatches->Matches(aURI); - } - - bool IsExtensionMatch(const URLInfo& aURI); - protected: virtual ~WebAccessibleResource() = default; private: MatchGlobSet mWebAccessiblePaths; RefPtr mMatches; - RefPtr mExtensions; }; class WebExtensionPolicy final : public nsISupports, diff --git a/toolkit/components/extensions/schemas/manifest.json b/toolkit/components/extensions/schemas/manifest.json index 0448ed0ca3bf..33c5f853154a 100644 --- a/toolkit/components/extensions/schemas/manifest.json +++ b/toolkit/components/extensions/schemas/manifest.json @@ -243,7 +243,6 @@ { "min_manifest_version": 3, "type": "array", - "postprocess": "webAcessibleMatching", "items": { "type": "object", "properties": { @@ -252,14 +251,8 @@ "items": { "type": "string" } }, "matches": { - "optional": true, "type": "array", "items": { "$ref": "MatchPatternRestricted" } - }, - "extensions": { - "optional": true, - "type": "array", - "items": { "$ref": "ExtensionID" } } } } diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_web_accessible_resources_matches.js b/toolkit/components/extensions/test/xpcshell/test_ext_web_accessible_resources_matches.js index cc531d668724..832e9966577a 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_web_accessible_resources_matches.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_web_accessible_resources_matches.js @@ -1,5 +1,4 @@ "use strict"; - Services.prefs.setBoolPref("extensions.manifestV3.enabled", true); const server = createHttpServer({ hosts: ["example.com", "example.org"] }); @@ -12,77 +11,10 @@ let image = atob( const IMAGE_ARRAYBUFFER = Uint8Array.from(image, byte => byte.charCodeAt(0)) .buffer; -add_task(async function test_web_accessible_resources_matching() { - let extension = await ExtensionTestUtils.loadExtension({ - manifest: { - manifest_version: 3, - web_accessible_resources: [ - { - resources: ["/accessible.html"], - }, - ], - }, - }); - - await Assert.rejects( - extension.startup(), - /web_accessible_resources requires one of "matches" or "extensions"/, - "web_accessible_resources object format incorrect" - ); - - extension = await ExtensionTestUtils.loadExtension({ - manifest: { - manifest_version: 3, - web_accessible_resources: [ - { - resources: ["/accessible.html"], - matches: ["http://example.com/data/*"], - }, - ], - }, - }); - - await extension.startup(); - ok(true, "web_accessible_resources with matches loads"); - await extension.unload(); - - extension = await ExtensionTestUtils.loadExtension({ - manifest: { - manifest_version: 3, - web_accessible_resources: [ - { - resources: ["/accessible.html"], - extensions: ["foo@mochitest"], - }, - ], - }, - }); - - await extension.startup(); - ok(true, "web_accessible_resources with extensions loads"); - await extension.unload(); - - extension = await ExtensionTestUtils.loadExtension({ - manifest: { - manifest_version: 3, - web_accessible_resources: [ - { - resources: ["/accessible.html"], - matches: ["http://example.com/data/*"], - extensions: ["foo@mochitest"], - }, - ], - }, - }); - - await extension.startup(); - ok(true, "web_accessible_resources with matches and extensions loads"); - await extension.unload(); -}); - add_task(async function test_web_accessible_resources() { async function contentScript() { let canLoad = window.location.href.startsWith("http://example.com"); + let urls = [ { name: "iframe", @@ -131,11 +63,7 @@ add_task(async function test_web_accessible_resources() { } for (let test of urls) { let loaded = await test_element_src(test.name, test.url); - browser.test.assertEq( - loaded, - test.shouldLoad, - `resource ${test.url} loaded` - ); + browser.test.assertEq(loaded, test.shouldLoad, "resource loaded"); } browser.test.notifyPass("web-accessible-resources"); } @@ -194,136 +122,3 @@ add_task(async function test_web_accessible_resources() { await page.close(); await extension.unload(); }); - -add_task(async function test_web_accessible_resources_extensions() { - async function pageScript() { - function test_element_src(data) { - return new Promise(resolve => { - let elem = document.createElement(data.elem); - let elemContext = data.content_context ? elem.wrappedJSObject : elem; - elemContext.setAttribute("src", data.url); - elem.addEventListener( - "load", - () => { - browser.test.log(`got load event for ${data.url}`); - resolve(true); - }, - { once: true } - ); - elem.addEventListener( - "error", - () => { - browser.test.log(`got error event for ${data.url}`); - resolve(false); - }, - { once: true } - ); - document.body.appendChild(elem); - }); - } - browser.test.onMessage.addListener(async msg => { - browser.test.log(`testing ${JSON.stringify(msg)}`); - let loaded = await test_element_src(msg); - browser.test.assertEq(loaded, msg.shouldLoad, `${msg.name} loaded`); - browser.test.sendMessage("web-accessible-resources"); - }); - } - - let other = ExtensionTestUtils.loadExtension({ - manifest: { - applications: { gecko: { id: "other@mochitest" } }, - }, - files: { - "page.js": pageScript, - - "page.html": ` - - - `, - }, - }); - - let extension = ExtensionTestUtils.loadExtension({ - manifest: { - manifest_version: 3, - applications: { gecko: { id: "this@mochitest" } }, - content_scripts: [ - { - matches: ["http://example.com/data/*"], - js: ["page.js"], - run_at: "document_idle", - }, - ], - web_accessible_resources: [ - { - resources: ["/image.png"], - extensions: ["other@mochitest"], - }, - ], - }, - - files: { - "image.png": IMAGE_ARRAYBUFFER, - "inaccessible.png": IMAGE_ARRAYBUFFER, - "page.js": pageScript, - - "page.html": ` - - - `, - }, - }); - - await extension.startup(); - let extensionUrl = `moz-extension://${extension.uuid}/`; - - await other.startup(); - let pageUrl = `moz-extension://${other.uuid}/page.html`; - - let page = await ExtensionTestUtils.loadContentPage(pageUrl); - - other.sendMessage({ - name: "accessible resource", - elem: "img", - url: `${extensionUrl}image.png`, - shouldLoad: true, - }); - await other.awaitMessage("web-accessible-resources"); - - other.sendMessage({ - name: "inaccessible resource", - elem: "img", - url: `${extensionUrl}inaccessible.png`, - shouldLoad: false, - }); - await other.awaitMessage("web-accessible-resources"); - - await page.close(); - - // test that the extension may load it's own web accessible resource - page = await ExtensionTestUtils.loadContentPage(`${extensionUrl}page.html`); - extension.sendMessage({ - name: "accessible resource", - elem: "img", - url: `${extensionUrl}image.png`, - shouldLoad: true, - }); - await extension.awaitMessage("web-accessible-resources"); - - await page.close(); - - // test that a web page not in matches cannot load the resource - page = await ExtensionTestUtils.loadContentPage("http://example.com/data/"); - - extension.sendMessage({ - name: "cannot access resource", - elem: "img", - url: `${extensionUrl}image.png`, - content_context: true, - shouldLoad: false, - }); - await extension.awaitMessage("web-accessible-resources"); - - await extension.unload(); - await other.unload(); -});