diff --git a/browser/actors/ContextMenuChild.jsm b/browser/actors/ContextMenuChild.jsm index 0bd1709b04e7..1158e676cb76 100644 --- a/browser/actors/ContextMenuChild.jsm +++ b/browser/actors/ContextMenuChild.jsm @@ -1137,7 +1137,7 @@ class ContextMenuChild extends JSWindowActorChild { try { if (elem.download) { // Ignore download attribute on cross-origin links - context.principal.checkMayLoad(context.linkURI, false, true); + context.principal.checkMayLoad(context.linkURI, true); context.linkDownload = elem.download; } } catch (ex) {} diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index ccbfdef2d41e..421798c6f0c4 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -349,9 +349,26 @@ BasePrincipal::SubsumesConsideringDomainIgnoringFPD(nsIPrincipal* aOther, } NS_IMETHODIMP -BasePrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, - bool aAllowIfInheritsPrincipal) { +BasePrincipal::CheckMayLoad(nsIURI* aURI, bool aAllowIfInheritsPrincipal) { + return CheckMayLoadHelper(aURI, aAllowIfInheritsPrincipal, false, 0); +} + +NS_IMETHODIMP +BasePrincipal::CheckMayLoadWithReporting(nsIURI* aURI, + bool aAllowIfInheritsPrincipal, + uint64_t aInnerWindowID) { + return CheckMayLoadHelper(aURI, aAllowIfInheritsPrincipal, true, + aInnerWindowID); +} + +nsresult BasePrincipal::CheckMayLoadHelper(nsIURI* aURI, + bool aAllowIfInheritsPrincipal, + bool aReport, + uint64_t aInnerWindowID) { NS_ENSURE_ARG_POINTER(aURI); + MOZ_ASSERT( + aReport || aInnerWindowID == 0, + "Why do we have an inner window id if we're not supposed to report?"); // Check the internal method first, which allows us to quickly approve loads // for the System Principal. @@ -385,7 +402,7 @@ BasePrincipal::CheckMayLoad(nsIURI* aURI, bool aReport, if (NS_SUCCEEDED(rv) && prinURI) { nsScriptSecurityManager::ReportError( "CheckSameOriginError", prinURI, aURI, - mOriginAttributes.mPrivateBrowsingId > 0); + mOriginAttributes.mPrivateBrowsingId > 0, aInnerWindowID); } } diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index 021f7ab21fd9..3b29971cf520 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -110,8 +110,10 @@ class BasePrincipal : public nsJSPrincipals { bool* _retval) final; NS_IMETHOD SubsumesConsideringDomainIgnoringFPD(nsIPrincipal* other, bool* _retval) final; - NS_IMETHOD CheckMayLoad(nsIURI* uri, bool report, - bool allowIfInheritsPrincipal) final; + NS_IMETHOD CheckMayLoad(nsIURI* uri, bool allowIfInheritsPrincipal) final; + NS_IMETHOD CheckMayLoadWithReporting(nsIURI* uri, + bool allowIfInheritsPrincipal, + uint64_t innerWindowID) final; NS_IMETHOD GetAddonPolicy(nsISupports** aResult) final; NS_IMETHOD GetIsNullPrincipal(bool* aResult) override; NS_IMETHOD GetIsContentPrincipal(bool* aResult) override; @@ -257,6 +259,10 @@ class BasePrincipal : public nsJSPrincipals { virtual bool MayLoadInternal(nsIURI* aURI) = 0; friend class ::ExpandedPrincipal; + // Helper for implementing CheckMayLoad and CheckMayLoadWithReporting. + nsresult CheckMayLoadHelper(nsIURI* aURI, bool aAllowIfInheritsPrincipal, + bool aReport, uint64_t aInnerWindowID); + void SetHasExplicitDomain() { mHasExplicitDomain = true; } // Either of these functions should be called as the last step of the diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index 8a8dee223128..feea6b4cd27e 100644 --- a/caps/nsIPrincipal.idl +++ b/caps/nsIPrincipal.idl @@ -140,16 +140,23 @@ interface nsIPrincipal : nsISerializable * * * @param uri The URI about to be loaded. - * @param report If true, will report a warning to the console service - * if the load is not allowed. * @param allowIfInheritsPrincipal If true, the load is allowed if the * loadee inherits the principal of the * loader. * @throws NS_ERROR_DOM_BAD_URI if the load is not allowed. */ - void checkMayLoad(in nsIURI uri, in boolean report, + void checkMayLoad(in nsIURI uri, in boolean allowIfInheritsPrincipal); + /** + * Like checkMayLoad, but if returning an error will also report that error + * to the console, using the provided window id. The window id may be 0 to + * report to just the browser console, not web consoles. + */ + void checkMayLoadWithReporting(in nsIURI uri, + in boolean allowIfInheritsPrincipal, + in unsigned long long innerWindowID); + /** * Checks if the provided URI is concidered third-party to the * URI of the principal. diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp index fdb5ac781800..725a99e575c9 100644 --- a/caps/nsScriptSecurityManager.cpp +++ b/caps/nsScriptSecurityManager.cpp @@ -676,7 +676,8 @@ nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal* aPrincipal, aPrincipal->OriginAttributesRef().mPrivateBrowsingId > 0); NS_ENSURE_SUCCESS(rv, rv); // Check the principal is allowed to load the target. - return aPrincipal->CheckMayLoad(targetBaseURI, true, false); + // Unfortunately we don't have a window id to work with here... + return aPrincipal->CheckMayLoadWithReporting(targetBaseURI, false, 0); } //-- get the source scheme diff --git a/devtools/client/webconsole/test/browser/browser.ini b/devtools/client/webconsole/test/browser/browser.ini index e5b23b11a459..17037be9fb01 100644 --- a/devtools/client/webconsole/test/browser/browser.ini +++ b/devtools/client/webconsole/test/browser/browser.ini @@ -141,6 +141,7 @@ support-files = test-reopen-closed-tab.html test-simple-function.html test-simple-function.js + test-same-origin-required-load.html test-sourcemap-error-01.html test-sourcemap-error-01.js test-sourcemap-error-02.html @@ -452,6 +453,7 @@ skip-if = fission [browser_webconsole_reverse_search_keyboard_navigation.js] [browser_webconsole_reverse_search_mouse_navigation.js] [browser_webconsole_reverse_search_toggle.js] +[browser_webconsole_same_origin_errors.js] [browser_webconsole_sandbox_update_after_navigation.js] fail-if = fission [browser_webconsole_script_errordoc_urls.js] diff --git a/devtools/client/webconsole/test/browser/browser_webconsole_same_origin_errors.js b/devtools/client/webconsole/test/browser/browser_webconsole_same_origin_errors.js new file mode 100644 index 000000000000..e5a50c3c9aa7 --- /dev/null +++ b/devtools/client/webconsole/test/browser/browser_webconsole_same_origin_errors.js @@ -0,0 +1,29 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Ensure that same-origin errors are logged to the console. + +"use strict"; + +const TEST_URI = + "http://example.com/browser/devtools/client/webconsole/test/browser/test-same-origin-required-load.html"; + +add_task(async function() { + const hud = await openNewTabAndConsole(TEST_URI); + + const targetURL = "http://example.org"; + const onErrorMessage = waitForMessage(hud, "may not load data"); + SpecialPowers.spawn(gBrowser.selectedBrowser, [targetURL], url => { + XPCNativeWrapper.unwrap(content).testTrack(url); + }); + const message = await onErrorMessage; + const node = message.node; + ok( + node.classList.contains("error"), + "The message has the expected classname" + ); + ok( + node.textContent.includes(targetURL), + "The message is about the thing we were expecting" + ); +}); diff --git a/devtools/client/webconsole/test/browser/test-same-origin-required-load.html b/devtools/client/webconsole/test/browser/test-same-origin-required-load.html new file mode 100644 index 000000000000..ba80eb956c7f --- /dev/null +++ b/devtools/client/webconsole/test/browser/test-same-origin-required-load.html @@ -0,0 +1,26 @@ + + + + + + Test loads that are required to be same-origin (no CORS involved) + + + + + diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 9eef2efe2e89..456fbddebd94 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -10886,8 +10886,8 @@ nsDocShell::AddState(JS::Handle aData, const nsAString& aTitle, // It's a file:// URI nsCOMPtr principal = document->GetPrincipal(); - if (!principal || - NS_FAILED(principal->CheckMayLoad(newURI, true, false))) { + if (!principal || NS_FAILED(principal->CheckMayLoadWithReporting( + newURI, false, document->InnerWindowID()))) { return NS_ERROR_DOM_SECURITY_ERR; } } diff --git a/dom/base/nsContentSink.cpp b/dom/base/nsContentSink.cpp index 3a23c486c2e4..72e5e76c6bf4 100644 --- a/dom/base/nsContentSink.cpp +++ b/dom/base/nsContentSink.cpp @@ -920,7 +920,7 @@ nsresult nsContentSink::SelectDocAppCache( } else { // The document was not loaded from an application cache // Here we know the manifest has the same origin as the - // document. There is call to CheckMayLoad() on it above. + // document. There is call to CheckMayLoadWithReporting() on it above. if (!aFetchedWithHTTPGetOrEquiv) { // The document was not loaded using HTTP GET or equivalent @@ -1058,7 +1058,8 @@ void nsContentSink::ProcessOfflineManifest(const nsAString& aManifestSpec) { } // Documents must list a manifest from the same origin - rv = mDocument->NodePrincipal()->CheckMayLoad(manifestURI, true, false); + rv = mDocument->NodePrincipal()->CheckMayLoadWithReporting( + manifestURI, false, mDocument->InnerWindowID()); if (NS_FAILED(rv)) { action = CACHE_SELECTION_RESELECT_WITHOUT_MANIFEST; } else { diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index 7e8a3db7cfa5..c1b9982d9ddb 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -5117,8 +5117,7 @@ void nsContentUtils::TriggerLink(nsIContent* aContent, nsIURI* aLinkURI, !aContent->IsSVGElement(nsGkAtoms::a)) || !aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::download, fileName) || - NS_FAILED( - aContent->NodePrincipal()->CheckMayLoad(aLinkURI, false, true))) { + NS_FAILED(aContent->NodePrincipal()->CheckMayLoad(aLinkURI, true))) { fileName.SetIsVoid(true); // No actionable download attribute was found. } @@ -5665,9 +5664,9 @@ nsresult nsContentUtils::CheckSameOrigin(nsIChannel* aOldChannel, NS_ENSURE_STATE(oldPrincipal && newURI && newOriginalURI); - nsresult rv = oldPrincipal->CheckMayLoad(newURI, false, false); + nsresult rv = oldPrincipal->CheckMayLoad(newURI, false); if (NS_SUCCEEDED(rv) && newOriginalURI != newURI) { - rv = oldPrincipal->CheckMayLoad(newOriginalURI, false, false); + rv = oldPrincipal->CheckMayLoad(newOriginalURI, false); } return rv; @@ -5814,7 +5813,7 @@ bool nsContentUtils::CheckMayLoad(nsIPrincipal* aPrincipal, NS_ENSURE_SUCCESS(rv, false); return NS_SUCCEEDED( - aPrincipal->CheckMayLoad(channelURI, false, aAllowIfInheritsPrincipal)); + aPrincipal->CheckMayLoad(channelURI, aAllowIfInheritsPrincipal)); } /* static */ @@ -6422,7 +6421,7 @@ bool nsContentUtils::ChannelShouldInheritPrincipal( // based on its own codebase later. // (URIIsLocalFile(aURI) && - NS_SUCCEEDED(aLoadingPrincipal->CheckMayLoad(aURI, false, false)) && + NS_SUCCEEDED(aLoadingPrincipal->CheckMayLoad(aURI, false)) && // One more check here. CheckMayLoad will always return true for the // system principal, but we do NOT want to inherit in that case. !aLoadingPrincipal->IsSystemPrincipal()); diff --git a/dom/cache/PrincipalVerifier.cpp b/dom/cache/PrincipalVerifier.cpp index 4ec2d0290004..c8fbc6c4c814 100644 --- a/dom/cache/PrincipalVerifier.cpp +++ b/dom/cache/PrincipalVerifier.cpp @@ -144,7 +144,7 @@ void PrincipalVerifier::VerifyOnMainThread() { DispatchToInitiatingThread(rv); return; } - rv = principal->CheckMayLoad(uri, false, false); + rv = principal->CheckMayLoad(uri, false); if (NS_WARN_IF(NS_FAILED(rv))) { DispatchToInitiatingThread(rv); return; diff --git a/dom/fetch/Request.cpp b/dom/fetch/Request.cpp index b0d62d147732..b79ee9ee4312 100644 --- a/dom/fetch/Request.cpp +++ b/dom/fetch/Request.cpp @@ -233,7 +233,7 @@ class ReferrerSameOriginChecker final : public WorkerMainThreadRunnable { if (NS_SUCCEEDED(NS_NewURI(getter_AddRefs(uri), mReferrerURL))) { nsCOMPtr principal = mWorkerPrivate->GetPrincipal(); if (principal) { - mResult = principal->CheckMayLoad(uri, /* report */ false, + mResult = principal->CheckMayLoad(uri, /* allowIfInheritsPrincipal */ false); } } @@ -362,7 +362,7 @@ already_AddRefed Request::Constructor(const GlobalObject& aGlobal, nsCOMPtr principal = global->PrincipalOrNull(); if (principal) { nsresult rv = - principal->CheckMayLoad(uri, /* report */ false, + principal->CheckMayLoad(uri, /* allowIfInheritsPrincipal */ false); if (NS_FAILED(rv)) { referrerURL.AssignLiteral(kFETCH_CLIENT_REFERRER_STR); diff --git a/dom/notification/Notification.cpp b/dom/notification/Notification.cpp index 3407fb56a125..f489e21013bb 100644 --- a/dom/notification/Notification.cpp +++ b/dom/notification/Notification.cpp @@ -295,7 +295,8 @@ class FocusWindowRunnable final : public Runnable { } }; -nsresult CheckScope(nsIPrincipal* aPrincipal, const nsACString& aScope) { +nsresult CheckScope(nsIPrincipal* aPrincipal, const nsACString& aScope, + uint64_t aWindowID) { AssertIsOnMainThread(); MOZ_ASSERT(aPrincipal); @@ -305,8 +306,9 @@ nsresult CheckScope(nsIPrincipal* aPrincipal, const nsACString& aScope) { return rv; } - return aPrincipal->CheckMayLoad(scopeURI, /* report = */ true, - /* allowIfInheritsPrincipal = */ false); + return aPrincipal->CheckMayLoadWithReporting( + scopeURI, + /* allowIfInheritsPrincipal = */ false, aWindowID); } } // anonymous namespace @@ -2075,7 +2077,7 @@ class CheckLoadRunnable final : public WorkerMainThreadRunnable { bool MainThreadRun() override { nsIPrincipal* principal = mWorkerPrivate->GetPrincipal(); - mRv = CheckScope(principal, mScope); + mRv = CheckScope(principal, mScope, mWorkerPrivate->WindowID()); if (NS_FAILED(mRv)) { return true; @@ -2120,7 +2122,13 @@ already_AddRefed Notification::ShowPersistentNotification( return nullptr; } - aRv = CheckScope(principal, NS_ConvertUTF16toUTF8(aScope)); + uint64_t windowID = 0; + nsCOMPtr win = do_QueryInterface(aGlobal); + if (win) { + windowID = win->WindowID(); + } + + aRv = CheckScope(principal, NS_ConvertUTF16toUTF8(aScope), windowID); if (NS_WARN_IF(aRv.Failed())) { aRv.Throw(NS_ERROR_DOM_SECURITY_ERR); return nullptr; diff --git a/dom/security/nsContentSecurityManager.cpp b/dom/security/nsContentSecurityManager.cpp index 398c37dc9a30..daa7f8519cb8 100644 --- a/dom/security/nsContentSecurityManager.cpp +++ b/dom/security/nsContentSecurityManager.cpp @@ -987,9 +987,9 @@ nsresult nsContentSecurityManager::CheckChannel(nsIChannel* aChannel) { nsIContentPolicy::TYPE_DOCUMENT); nsIPrincipal* loadingPrincipal = loadInfo->LoadingPrincipal(); - // It doesn't matter what we pass for the third, data-inherits, argument. + // It doesn't matter what we pass for the second, data-inherits, argument. // Any protocol which inherits won't pay attention to cookies anyway. - rv = loadingPrincipal->CheckMayLoad(uri, false, false); + rv = loadingPrincipal->CheckMayLoad(uri, false); if (NS_FAILED(rv)) { AddLoadFlags(aChannel, nsIRequest::LOAD_ANONYMOUS); } diff --git a/dom/serviceworkers/ServiceWorkerManager.cpp b/dom/serviceworkers/ServiceWorkerManager.cpp index 10d722494968..02e5f8e420ee 100644 --- a/dom/serviceworkers/ServiceWorkerManager.cpp +++ b/dom/serviceworkers/ServiceWorkerManager.cpp @@ -910,8 +910,12 @@ class GetRegistrationsRunnable final : public Runnable { break; } - rv = principal->CheckMayLoad(scopeURI, true /* report */, - false /* allowIfInheritsPrincipal */); + // Unfortunately we don't seem to have an obvious window id here; in + // particular ClientInfo does not have one, and neither do service worker + // registrations, as far as I can tell. + rv = principal->CheckMayLoadWithReporting( + scopeURI, false /* allowIfInheritsPrincipal */, + 0 /* innerWindowID */); if (NS_WARN_IF(NS_FAILED(rv))) { continue; } @@ -973,8 +977,11 @@ class GetRegistrationRunnable final : public Runnable { return NS_OK; } - rv = principal->CheckMayLoad(uri, true /* report */, - false /* allowIfInheritsPrinciple */); + // Unfortunately we don't seem to have an obvious window id here; in + // particular ClientInfo does not have one, and neither do service worker + // registrations, as far as I can tell. + rv = principal->CheckMayLoadWithReporting( + uri, false /* allowIfInheritsPrincipal */, 0 /* innerWindowID */); if (NS_FAILED(rv)) { mPromise->Reject(NS_ERROR_DOM_SECURITY_ERR, __func__); return NS_OK; diff --git a/dom/serviceworkers/ServiceWorkerUtils.cpp b/dom/serviceworkers/ServiceWorkerUtils.cpp index 982241675868..e89d57bd736c 100644 --- a/dom/serviceworkers/ServiceWorkerUtils.cpp +++ b/dom/serviceworkers/ServiceWorkerUtils.cpp @@ -86,12 +86,14 @@ nsresult ServiceWorkerScopeAndScriptAreValid(const ClientInfo& aClientInfo, Unused << aScriptURI->GetRef(ref); NS_ENSURE_TRUE(ref.IsEmpty(), NS_ERROR_DOM_SECURITY_ERR); - rv = principal->CheckMayLoad(aScopeURI, true /* report */, - false /* allowIfInheritsPrincipal */); + // Unfortunately we don't seem to have an obvious window id here; in + // particular ClientInfo does not have one. + rv = principal->CheckMayLoadWithReporting( + aScopeURI, false /* allowIfInheritsPrincipal */, 0 /* innerWindowID */); NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_SECURITY_ERR); - rv = principal->CheckMayLoad(aScriptURI, true /* report */, - false /* allowIfInheritsPrincipal */); + rv = principal->CheckMayLoadWithReporting( + aScriptURI, false /* allowIfInheritsPrincipal */, 0 /* innerWindowID */); NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_SECURITY_ERR); return NS_OK; diff --git a/netwerk/base/nsNetUtil.cpp b/netwerk/base/nsNetUtil.cpp index f8bf805e367e..c9b36c76ef27 100644 --- a/netwerk/base/nsNetUtil.cpp +++ b/netwerk/base/nsNetUtil.cpp @@ -1986,6 +1986,8 @@ bool NS_HasBeenCrossOrigin(nsIChannel* aChannel, bool aReport) { bool aboutBlankInherits = dataInherits && loadInfo->GetAboutBlankInherits(); + uint64_t innerWindowID = loadInfo->GetInnerWindowID(); + for (nsIRedirectHistoryEntry* redirectHistoryEntry : loadInfo->RedirectChain()) { nsCOMPtr principal; @@ -2004,7 +2006,14 @@ bool NS_HasBeenCrossOrigin(nsIChannel* aChannel, bool aReport) { continue; } - if (NS_FAILED(loadingPrincipal->CheckMayLoad(uri, aReport, dataInherits))) { + nsresult res; + if (aReport) { + res = loadingPrincipal->CheckMayLoadWithReporting(uri, dataInherits, + innerWindowID); + } else { + res = loadingPrincipal->CheckMayLoad(uri, dataInherits); + } + if (NS_FAILED(res)) { return true; } } @@ -2019,7 +2028,15 @@ bool NS_HasBeenCrossOrigin(nsIChannel* aChannel, bool aReport) { return false; } - return NS_FAILED(loadingPrincipal->CheckMayLoad(uri, aReport, dataInherits)); + nsresult res; + if (aReport) { + res = loadingPrincipal->CheckMayLoadWithReporting(uri, dataInherits, + innerWindowID); + } else { + res = loadingPrincipal->CheckMayLoad(uri, dataInherits); + } + + return NS_FAILED(res); } bool NS_IsSafeTopLevelNav(nsIChannel* aChannel) { diff --git a/netwerk/protocol/http/nsCORSListenerProxy.cpp b/netwerk/protocol/http/nsCORSListenerProxy.cpp index 7ad49f3bf4c5..7ea193162a61 100644 --- a/netwerk/protocol/http/nsCORSListenerProxy.cpp +++ b/netwerk/protocol/http/nsCORSListenerProxy.cpp @@ -914,9 +914,9 @@ nsresult nsCORSListenerProxy::UpdateChannel(nsIChannel* aChannel, } if (!mHasBeenCrossSite && - NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(uri, false, false)) && - (originalURI == uri || NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad( - originalURI, false, false)))) { + NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(uri, false)) && + (originalURI == uri || + NS_SUCCEEDED(mRequestingPrincipal->CheckMayLoad(originalURI, false)))) { return NS_OK; }