From 963f23d8caf3397020a9ffff1dc64349ce36680e Mon Sep 17 00:00:00 2001 From: Tanvi Vyas Date: Tue, 24 Mar 2015 09:18:48 -0700 Subject: [PATCH] Bug 1082837 - Call content policies on cached image redirects in imgLoader::ValidateSecurityInfo. Content policies check the last hop (final uri) of the cached image. For Mixed Content Blocker, we do an additional check to see if any of the intermediary hops went through an insecure redirect. r=smaug, feedback=seth --- dom/security/nsMixedContentBlocker.cpp | 46 +++++++++++++-- dom/security/nsMixedContentBlocker.h | 21 +++++++ image/src/imgLoader.cpp | 78 ++++++++++++++++++++++++-- 3 files changed, 137 insertions(+), 8 deletions(-) diff --git a/dom/security/nsMixedContentBlocker.cpp b/dom/security/nsMixedContentBlocker.cpp index b5a5ee47491..b0aede46838 100644 --- a/dom/security/nsMixedContentBlocker.cpp +++ b/dom/security/nsMixedContentBlocker.cpp @@ -286,7 +286,11 @@ nsMixedContentBlocker::AsyncOnChannelRedirect(nsIChannel* aOldChannel, return NS_OK; } -NS_IMETHODIMP +/* This version of ShouldLoad() is non-static and called by the Content Policy + * API and AsyncOnChannelRedirect(). See nsIContentPolicy::ShouldLoad() + * for detailed description of the parameters. + */ +nsresult nsMixedContentBlocker::ShouldLoad(uint32_t aContentType, nsIURI* aContentLocation, nsIURI* aRequestingLocation, @@ -295,6 +299,36 @@ nsMixedContentBlocker::ShouldLoad(uint32_t aContentType, nsISupports* aExtra, nsIPrincipal* aRequestPrincipal, int16_t* aDecision) +{ + // We pass in false as the first parameter to ShouldLoad(), because the + // callers of this method don't know whether the load went through cached + // image redirects. This is handled by direct callers of the static + // ShouldLoad. + nsresult rv = ShouldLoad(false, //aHadInsecureImageRedirect + aContentType, + aContentLocation, + aRequestingLocation, + aRequestingContext, + aMimeGuess, + aExtra, + aRequestPrincipal, + aDecision); + return rv; +} + +/* Static version of ShouldLoad() that contains all the Mixed Content Blocker + * logic. Called from non-static ShouldLoad(). + */ +NS_IMETHODIMP +nsMixedContentBlocker::ShouldLoad(bool aHadInsecureImageRedirect, + uint32_t aContentType, + nsIURI* aContentLocation, + nsIURI* aRequestingLocation, + nsISupports* aRequestingContext, + const nsACString& aMimeGuess, + nsISupports* aExtra, + nsIPrincipal* aRequestPrincipal, + int16_t* aDecision) { // Asserting that we are on the main thread here and hence do not have to lock // and unlock sBlockMixedScript and sBlockMixedDisplay before reading/writing @@ -442,8 +476,12 @@ nsMixedContentBlocker::ShouldLoad(uint32_t aContentType, *aDecision = REJECT_REQUEST; return NS_ERROR_FAILURE; } - - if (schemeLocal || schemeNoReturnData || schemeInherits || schemeSecure) { + // TYPE_IMAGE redirects are cached based on the original URI, not the final + // destination and hence cache hits for images may not have the correct + // aContentLocation. Check if the cached hit went through an http redirect, + // and if it did, we can't treat this as a secure subresource. + if (!aHadInsecureImageRedirect && + (schemeLocal || schemeNoReturnData || schemeInherits || schemeSecure)) { *aDecision = ACCEPT; return NS_OK; } @@ -544,7 +582,7 @@ nsMixedContentBlocker::ShouldLoad(uint32_t aContentType, rv = docShell->GetAllowMixedContentAndConnectionData(&rootHasSecureConnection, &allowMixedContent, &isRootDocShell); if (NS_FAILED(rv)) { *aDecision = REJECT_REQUEST; - return rv; + return rv; } diff --git a/dom/security/nsMixedContentBlocker.h b/dom/security/nsMixedContentBlocker.h index b162670e187..8309bbfc7c5 100644 --- a/dom/security/nsMixedContentBlocker.h +++ b/dom/security/nsMixedContentBlocker.h @@ -25,10 +25,12 @@ enum MixedContentTypes { #include "nsIContentPolicy.h" #include "nsIChannel.h" #include "nsIChannelEventSink.h" +#include "imgRequest.h" class nsMixedContentBlocker : public nsIContentPolicy, public nsIChannelEventSink { +private: virtual ~nsMixedContentBlocker(); public: @@ -37,6 +39,25 @@ public: NS_DECL_NSICHANNELEVENTSINK nsMixedContentBlocker(); + + /* Static version of ShouldLoad() that contains all the Mixed Content Blocker + * logic. Called from non-static ShouldLoad(). + * Called directly from imageLib when an insecure redirect exists in a cached + * image load. + * @param aHadInsecureImageRedirect + * boolean flag indicating that an insecure redirect through http + * occured when this image was initially loaded and cached. + * Remaining parameters are from nsIContentPolicy::ShouldLoad(). + */ + static nsresult ShouldLoad(bool aHadInsecureImageRedirect, + uint32_t aContentType, + nsIURI* aContentLocation, + nsIURI* aRequestingLocation, + nsISupports* aRequestingContext, + const nsACString& aMimeGuess, + nsISupports* aExtra, + nsIPrincipal* aRequestPrincipal, + int16_t* aDecision); static bool sBlockMixedScript; static bool sBlockMixedDisplay; }; diff --git a/image/src/imgLoader.cpp b/image/src/imgLoader.cpp index 56a96567446..8fa7d85205d 100644 --- a/image/src/imgLoader.cpp +++ b/image/src/imgLoader.cpp @@ -15,6 +15,7 @@ #include "nsCOMPtr.h" +#include "nsContentPolicyUtils.h" #include "nsContentUtils.h" #include "nsCORSListenerProxy.h" #include "nsNetUtil.h" @@ -29,6 +30,7 @@ #include "nsIFileURL.h" #include "nsCRT.h" #include "nsINetworkPredictor.h" +#include "mozilla/dom/nsMixedContentBlocker.h" #include "nsIApplicationCache.h" #include "nsIApplicationCacheContainer.h" @@ -588,6 +590,71 @@ static bool ShouldRevalidateEntry(imgCacheEntry *aEntry, return bValidateEntry; } +/* Call content policies on cached images that went through a redirect */ +static bool +ShouldLoadCachedImage(imgRequest* aImgRequest, + nsISupports* aLoadingContext, + nsIPrincipal* aLoadingPrincipal) +{ + /* Call content policies on cached images - Bug 1082837 + * Cached images are keyed off of the first uri in a redirect chain. + * Hence content policies don't get a chance to test the intermediate hops + * or the final desitnation. Here we test the final destination using + * mCurrentURI off of the imgRequest and passing it into content policies. + * For Mixed Content Blocker, we do an additional check to determine if any + * of the intermediary hops went through an insecure redirect with the + * mHadInsecureRedirect flag + */ + bool insecureRedirect = aImgRequest->HadInsecureRedirect(); + nsCOMPtr contentLocation; + aImgRequest->GetCurrentURI(getter_AddRefs(contentLocation)); + nsresult rv; + + int16_t decision = nsIContentPolicy::REJECT_REQUEST; + rv = NS_CheckContentLoadPolicy(nsIContentPolicy::TYPE_IMAGE, + contentLocation, + aLoadingPrincipal, + aLoadingContext, + EmptyCString(), //mime guess + nullptr, //aExtra + &decision, + nsContentUtils::GetContentPolicy(), + nsContentUtils::GetSecurityManager()); + if (NS_FAILED(rv) || !NS_CP_ACCEPTED(decision)) { + return false; + } + + // We call all Content Policies above, but we also have to call mcb + // individually to check the intermediary redirect hops are secure. + if (insecureRedirect) { + if (!nsContentUtils::IsSystemPrincipal(aLoadingPrincipal)) { + // Set the requestingLocation from the aLoadingPrincipal. + nsCOMPtr requestingLocation; + if (aLoadingPrincipal) { + rv = aLoadingPrincipal->GetURI(getter_AddRefs(requestingLocation)); + NS_ENSURE_SUCCESS(rv, false); + } + + // reset the decision for mixed content blocker check + decision = nsIContentPolicy::REJECT_REQUEST; + rv = nsMixedContentBlocker::ShouldLoad(insecureRedirect, + nsIContentPolicy::TYPE_IMAGE, + contentLocation, + requestingLocation, + aLoadingContext, + EmptyCString(), //mime guess + nullptr, + aLoadingPrincipal, + &decision); + if (NS_FAILED(rv) || !NS_CP_ACCEPTED(decision)) { + return false; + } + } + } + + return true; +} + // Returns true if this request is compatible with the given CORS mode on the // given loading principal, and false if the request may not be reused due // to CORS. Also checks the Referrer Policy, since requests with different @@ -595,7 +662,7 @@ static bool ShouldRevalidateEntry(imgCacheEntry *aEntry, static bool ValidateSecurityInfo(imgRequest* request, bool forcePrincipalCheck, int32_t corsmode, nsIPrincipal* loadingPrincipal, - ReferrerPolicy referrerPolicy) + nsISupports* aCX, ReferrerPolicy referrerPolicy) { // If the entry's Referrer Policy doesn't match, we can't use this request. if (referrerPolicy != request->GetReferrerPolicy()) { @@ -619,11 +686,14 @@ ValidateSecurityInfo(imgRequest* request, bool forcePrincipalCheck, if (otherprincipal && loadingPrincipal) { bool equals = false; otherprincipal->Equals(loadingPrincipal, &equals); - return equals; + if (!equals) { + return false; + } } } - return true; + // Content Policy Check on Cached Images + return ShouldLoadCachedImage(request, aCX, loadingPrincipal); } static nsresult NewImageChannel(nsIChannel **aResult, @@ -1604,7 +1674,7 @@ bool imgLoader::ValidateEntry(imgCacheEntry *aEntry, if (!ValidateSecurityInfo(request, aEntry->ForcePrincipalCheck(), aCORSMode, aLoadingPrincipal, - aReferrerPolicy)) + aCX, aReferrerPolicy)) return false; // data URIs are immutable and by their nature can't leak data, so we can