From 685321b74670296a8e1356dc17506d95369144d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 17 Feb 2022 18:11:58 +0000 Subject: [PATCH] Bug 1755947 - Simplify nsIPrincipal.isSameOrigin(). r=bholley Differential Revision: https://phabricator.services.mozilla.com/D139030 --- caps/BasePrincipal.cpp | 10 ++++++--- caps/BasePrincipal.h | 3 +-- caps/ContentPrincipal.cpp | 6 +----- caps/nsIPrincipal.idl | 15 ++++++------- dom/base/Document.cpp | 6 +----- dom/security/FramingChecker.cpp | 7 +------ dom/security/ReferrerInfo.cpp | 9 +------- dom/security/SecFetch.cpp | 21 ++++++------------- dom/workers/ScriptLoader.cpp | 6 +----- dom/workers/WorkerLoadInfo.cpp | 5 +---- netwerk/protocol/http/HttpBaseChannel.cpp | 13 ++---------- .../http/nsHttpChannelAuthProvider.cpp | 4 +--- 12 files changed, 31 insertions(+), 74 deletions(-) diff --git a/caps/BasePrincipal.cpp b/caps/BasePrincipal.cpp index 342467d0fb3a..60f11c75d000 100644 --- a/caps/BasePrincipal.cpp +++ b/caps/BasePrincipal.cpp @@ -658,19 +658,23 @@ BasePrincipal::IsThirdPartyChannel(nsIChannel* aChan, bool* aRes) { } NS_IMETHODIMP -BasePrincipal::IsSameOrigin(nsIURI* aURI, bool aIsPrivateWin, bool* aRes) { +BasePrincipal::IsSameOrigin(nsIURI* aURI, bool* aRes) { *aRes = false; nsCOMPtr prinURI; nsresult rv = GetURI(getter_AddRefs(prinURI)); if (NS_FAILED(rv) || !prinURI) { + // Note that expanded and system principals return here, because they have + // no URI. return NS_OK; } nsIScriptSecurityManager* ssm = nsContentUtils::GetSecurityManager(); if (!ssm) { - return NS_ERROR_UNEXPECTED; + return NS_OK; } + bool reportError = false; + bool isPrivateWindow = false; // Only used for error reporting. *aRes = NS_SUCCEEDED( - ssm->CheckSameOriginURI(prinURI, aURI, false, aIsPrivateWin)); + ssm->CheckSameOriginURI(prinURI, aURI, reportError, isPrivateWindow)); return NS_OK; } diff --git a/caps/BasePrincipal.h b/caps/BasePrincipal.h index e8835418b5d9..4d13cf2e5fb0 100644 --- a/caps/BasePrincipal.h +++ b/caps/BasePrincipal.h @@ -161,8 +161,7 @@ class BasePrincipal : public nsJSPrincipals { NS_IMETHOD IsThirdPartyPrincipal(nsIPrincipal* uri, bool* aRes) override; NS_IMETHOD IsThirdPartyChannel(nsIChannel* aChannel, bool* aRes) override; NS_IMETHOD GetIsOriginPotentiallyTrustworthy(bool* aResult) override; - NS_IMETHOD IsSameOrigin(nsIURI* aURI, bool aIsPrivateWin, - bool* aRes) override; + NS_IMETHOD IsSameOrigin(nsIURI* aURI, bool* aRes) override; NS_IMETHOD GetPrefLightCacheKey(nsIURI* aURI, bool aWithCredentials, const OriginAttributes& aOriginAttributes, nsACString& _retval) override; diff --git a/caps/ContentPrincipal.cpp b/caps/ContentPrincipal.cpp index aee023d3546e..081de92c573a 100644 --- a/caps/ContentPrincipal.cpp +++ b/caps/ContentPrincipal.cpp @@ -229,7 +229,6 @@ bool ContentPrincipal::SubsumesInternal( // explicitly setting document.domain then the other must also have // done so in order to be considered the same origin. This prevents // DNS spoofing based on document.domain (154930) - nsresult rv; if (aConsideration == ConsiderDocumentDomain) { // Get .domain on each principal. nsCOMPtr thisDomain, otherDomain; @@ -256,10 +255,7 @@ bool ContentPrincipal::SubsumesInternal( } // Compare uris. - bool isSameOrigin = false; - rv = aOther->IsSameOrigin(mURI, false, &isSameOrigin); - NS_ENSURE_SUCCESS(rv, false); - return isSameOrigin; + return aOther->IsSameOrigin(mURI); } NS_IMETHODIMP diff --git a/caps/nsIPrincipal.idl b/caps/nsIPrincipal.idl index 63855348a301..b881f51dff8b 100644 --- a/caps/nsIPrincipal.idl +++ b/caps/nsIPrincipal.idl @@ -331,7 +331,8 @@ interface nsIPrincipal : nsISupports * Uses NS_Security Compare to determine if the * other URI is same-origin as the uri of the Principal */ - bool isSameOrigin(in nsIURI otherURI, in bool aIsPrivateWin); + [infallible] + boolean isSameOrigin(in nsIURI otherURI); /* * Checks if the Principal is allowed to load the Provided file:// URI @@ -349,16 +350,16 @@ interface nsIPrincipal : nsISupports /* - * Checks if the Principals URI has first party storage access - * when loaded inside the provided 3rd party resource window. + * Checks if the Principals URI has first party storage access + * when loaded inside the provided 3rd party resource window. * See also: ContentBlocking::ShouldAllowAccessFor */ bool hasFirstpartyStorageAccess(in mozIDOMWindow aWindow, out uint32_t rejectedReason); - - + + /* - * Returns a Key for the LocalStorage Manager, used to - * check the Principals Origin Storage usage. + * Returns a Key for the LocalStorage Manager, used to + * check the Principals Origin Storage usage. */ readonly attribute ACString localStorageQuotaKey; diff --git a/dom/base/Document.cpp b/dom/base/Document.cpp index ccb50f06c828..2d8bedf40433 100644 --- a/dom/base/Document.cpp +++ b/dom/base/Document.cpp @@ -17089,11 +17089,7 @@ already_AddRefed Document::RequestStorageAccessForOrigin( // Only enforce third-party checks when there is a reason to enforce them. if (!CookieJarSettings()->GetRejectThirdPartyContexts()) { // If the the thrid party origin is equal to the window's, resolve. - bool isSameOrigin = false; - NodePrincipal()->IsSameOrigin(thirdPartyURI, - nsContentUtils::IsInPrivateBrowsing(this), - &isSameOrigin); - if (isSameOrigin) { + if (NodePrincipal()->IsSameOrigin(thirdPartyURI)) { promise->MaybeResolveWithUndefined(); return promise.forget(); } diff --git a/dom/security/FramingChecker.cpp b/dom/security/FramingChecker.cpp index f4e400659183..be82ac9bb3e6 100644 --- a/dom/security/FramingChecker.cpp +++ b/dom/security/FramingChecker.cpp @@ -104,12 +104,7 @@ bool FramingChecker::CheckOneFrameOptionsPolicy(nsIHttpChannel* aHttpChannel, } if (checkSameOrigin) { - bool isPrivateWin = false; - bool isSameOrigin = false; - if (principal) { - isPrivateWin = principal->OriginAttributesRef().mPrivateBrowsingId > 0; - principal->IsSameOrigin(uri, isPrivateWin, &isSameOrigin); - } + bool isSameOrigin = principal && principal->IsSameOrigin(uri); // one of the ancestors is not same origin as this document if (!isSameOrigin) { ReportError("XFrameOptionsDeny", aHttpChannel, uri, aPolicy); diff --git a/dom/security/ReferrerInfo.cpp b/dom/security/ReferrerInfo.cpp index 95f839e80028..bcd58321bcca 100644 --- a/dom/security/ReferrerInfo.cpp +++ b/dom/security/ReferrerInfo.cpp @@ -497,14 +497,7 @@ bool ReferrerInfo::IsCrossOriginRequest(nsIHttpChannel* aChannel) { return true; } - bool isPrivateWin = loadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; - bool isSameOrigin = false; - rv = loadInfo->TriggeringPrincipal()->IsSameOrigin(uri, isPrivateWin, - &isSameOrigin); - if (NS_WARN_IF(NS_FAILED(rv))) { - return true; - } - return !isSameOrigin; + return !loadInfo->TriggeringPrincipal()->IsSameOrigin(uri); } /* static */ diff --git a/dom/security/SecFetch.cpp b/dom/security/SecFetch.cpp index 817eb33c2b64..7c18e60d0345 100644 --- a/dom/security/SecFetch.cpp +++ b/dom/security/SecFetch.cpp @@ -118,7 +118,7 @@ nsCString MapInternalContentPolicyTypeToDest(nsContentPolicyType aType) { // a URI in the sec-fetch context. void IsExpandedPrincipalSameOrigin( nsCOMPtr aExpandedPrincipal, nsIURI* aURI, - bool aIsPrivateWin, bool* aRes) { + bool* aRes) { *aRes = false; for (const auto& principal : aExpandedPrincipal->AllowList()) { // Ignore extension principals to continue treating @@ -126,8 +126,7 @@ void IsExpandedPrincipalSameOrigin( if (!mozilla::BasePrincipal::Cast(principal)->AddonPolicy()) { // A ExpandedPrincipal usually has at most one ContentPrincipal, so we can // check IsSameOrigin on it here and return early. - mozilla::BasePrincipal::Cast(principal)->IsSameOrigin(aURI, aIsPrivateWin, - aRes); + mozilla::BasePrincipal::Cast(principal)->IsSameOrigin(aURI, aRes); return; } } @@ -149,16 +148,13 @@ bool IsSameOrigin(nsIHttpChannel* aHTTPChannel) { ->AddonAllowsLoad(channelURI); } - bool isPrivateWin = loadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; bool isSameOrigin = false; if (nsContentUtils::IsExpandedPrincipal(loadInfo->TriggeringPrincipal())) { nsCOMPtr ep = do_QueryInterface(loadInfo->TriggeringPrincipal()); - IsExpandedPrincipalSameOrigin(ep, channelURI, isPrivateWin, &isSameOrigin); + IsExpandedPrincipalSameOrigin(ep, channelURI, &isSameOrigin); } else { - nsresult rv = loadInfo->TriggeringPrincipal()->IsSameOrigin( - channelURI, isPrivateWin, &isSameOrigin); - mozilla::Unused << NS_WARN_IF(NS_FAILED(rv)); + isSameOrigin = loadInfo->TriggeringPrincipal()->IsSameOrigin(channelURI); } // if the initial request is not same-origin, we can return here @@ -172,13 +168,8 @@ bool IsSameOrigin(nsIHttpChannel* aHTTPChannel) { nsCOMPtr redirectPrincipal; for (nsIRedirectHistoryEntry* entry : loadInfo->RedirectChain()) { entry->GetPrincipal(getter_AddRefs(redirectPrincipal)); - if (redirectPrincipal) { - nsresult rv = redirectPrincipal->IsSameOrigin(channelURI, isPrivateWin, - &isSameOrigin); - mozilla::Unused << NS_WARN_IF(NS_FAILED(rv)); - if (!isSameOrigin) { - return false; - } + if (redirectPrincipal && !redirectPrincipal->IsSameOrigin(channelURI)) { + return false; } } diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 512d0499a99d..8d6595535af9 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1295,11 +1295,7 @@ class ScriptLoaderRunnable final : public nsIRunnable, public nsINamed { rv = NS_GetFinalChannelURI(channel, getter_AddRefs(finalURI)); NS_ENSURE_SUCCESS(rv, rv); - bool isSameOrigin = false; - rv = principal->IsSameOrigin(finalURI, false, &isSameOrigin); - NS_ENSURE_SUCCESS(rv, rv); - - if (isSameOrigin) { + if (principal->IsSameOrigin(finalURI)) { nsCString filename; rv = finalURI->GetSpec(filename); NS_ENSURE_SUCCESS(rv, rv); diff --git a/dom/workers/WorkerLoadInfo.cpp b/dom/workers/WorkerLoadInfo.cpp index 4ace4b7f3471..0121e9a39ccb 100644 --- a/dom/workers/WorkerLoadInfo.cpp +++ b/dom/workers/WorkerLoadInfo.cpp @@ -340,10 +340,7 @@ bool WorkerLoadInfo::PrincipalURIMatchesScriptURL() { return true; } - bool isSameOrigin = false; - rv = mPrincipal->IsSameOrigin(mBaseURI, false, &isSameOrigin); - - if (NS_SUCCEEDED(rv) && isSameOrigin) { + if (mPrincipal->IsSameOrigin(mBaseURI)) { return true; } diff --git a/netwerk/protocol/http/HttpBaseChannel.cpp b/netwerk/protocol/http/HttpBaseChannel.cpp index 1e489c81ca8c..4066c8921048 100644 --- a/netwerk/protocol/http/HttpBaseChannel.cpp +++ b/netwerk/protocol/http/HttpBaseChannel.cpp @@ -2620,11 +2620,7 @@ nsresult EnsureMIMEOfScript(HttpBaseChannel* aChannel, nsIURI* aURI, break; } - bool isPrivateWin = aLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; - bool isSameOrigin = false; - aLoadInfo->GetLoadingPrincipal()->IsSameOrigin(aURI, isPrivateWin, - &isSameOrigin); - if (isSameOrigin) { + if (aLoadInfo->GetLoadingPrincipal()->IsSameOrigin(aURI)) { // same origin AccumulateCategorical( Telemetry::LABELS_SCRIPT_BLOCK_INCORRECT_MIME_3::same_origin); @@ -2640,12 +2636,7 @@ nsresult EnsureMIMEOfScript(HttpBaseChannel* aChannel, nsIURI* aURI, nsCOMPtr corsOriginURI; rv = NS_NewURI(getter_AddRefs(corsOriginURI), corsOrigin); if (NS_SUCCEEDED(rv)) { - bool isPrivateWin = - aLoadInfo->GetOriginAttributes().mPrivateBrowsingId > 0; - bool isSameOrigin = false; - aLoadInfo->GetLoadingPrincipal()->IsSameOrigin( - corsOriginURI, isPrivateWin, &isSameOrigin); - if (isSameOrigin) { + if (aLoadInfo->GetLoadingPrincipal()->IsSameOrigin(corsOriginURI)) { cors = true; } } diff --git a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp index d188f0553d04..8c189ef35234 100644 --- a/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp +++ b/netwerk/protocol/http/nsHttpChannelAuthProvider.cpp @@ -1085,9 +1085,7 @@ bool nsHttpChannelAuthProvider::BlockPrompt(bool proxyAuth) { } else { nsIPrincipal* loadingPrinc = loadInfo->GetLoadingPrincipal(); MOZ_ASSERT(loadingPrinc); - bool sameOrigin = false; - loadingPrinc->IsSameOrigin(mURI, false, &sameOrigin); - mCrossOrigin = !sameOrigin; + mCrossOrigin = !loadingPrinc->IsSameOrigin(mURI); } }