From f4c9f068fa7dbe6745955721dc5666208e92e381 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Tue, 4 Jun 2019 06:32:37 +0000 Subject: [PATCH] Bug 1554847 - Improve cross-origin checks in canvas API - consider intermediate redirects, r=jya Differential Revision: https://phabricator.services.mozilla.com/D32792 --HG-- extra : moz-landing-system : lando --- dom/canvas/CanvasRenderingContext2D.cpp | 8 +++++++- dom/canvas/CanvasUtils.cpp | 10 +++++++--- dom/canvas/CanvasUtils.h | 3 ++- dom/canvas/ImageBitmap.cpp | 4 +++- dom/html/HTMLMediaElement.cpp | 7 +++++++ dom/html/HTMLMediaElement.h | 4 ++++ dom/media/BaseMediaResource.h | 4 ++++ dom/media/ChannelMediaDecoder.cpp | 5 +++++ dom/media/ChannelMediaDecoder.h | 1 + dom/media/ChannelMediaResource.cpp | 20 +++++++++++++++++++ dom/media/ChannelMediaResource.h | 5 +++++ dom/media/CloneableWithRangeMediaResource.cpp | 14 +++++++++++++ dom/media/CloneableWithRangeMediaResource.h | 1 + dom/media/FileMediaResource.cpp | 15 ++++++++++++++ dom/media/FileMediaResource.h | 1 + dom/media/MediaDecoder.h | 4 ++++ dom/media/hls/HLSDecoder.cpp | 6 ++++++ dom/media/hls/HLSDecoder.h | 1 + dom/media/mediasource/MediaSourceDecoder.cpp | 5 +++++ dom/media/mediasource/MediaSourceDecoder.h | 2 ++ layout/base/nsLayoutUtils.cpp | 17 +++++++++++----- layout/base/nsLayoutUtils.h | 3 +++ 22 files changed, 129 insertions(+), 11 deletions(-) diff --git a/dom/canvas/CanvasRenderingContext2D.cpp b/dom/canvas/CanvasRenderingContext2D.cpp index 7e1533e48e8b..60a9b2fe9401 100644 --- a/dom/canvas/CanvasRenderingContext2D.cpp +++ b/dom/canvas/CanvasRenderingContext2D.cpp @@ -4238,6 +4238,11 @@ CanvasRenderingContext2D::CachedSurfaceFromElement(Element* aElement) { return res; } + if (NS_FAILED(imgRequest->GetHadCrossOriginRedirects( + &res.mHadCrossOriginRedirects))) { + return res; + } + res.mSourceSurface = CanvasImageCache::LookupAllCanvas(aElement); if (!res.mSourceSurface) { return res; @@ -4251,7 +4256,8 @@ CanvasRenderingContext2D::CachedSurfaceFromElement(Element* aElement) { res.mSize = res.mSourceSurface->GetSize(); res.mPrincipal = principal.forget(); res.mImageRequest = imgRequest.forget(); - res.mIsWriteOnly = CheckWriteOnlySecurity(res.mCORSUsed, res.mPrincipal); + res.mIsWriteOnly = CheckWriteOnlySecurity(res.mCORSUsed, res.mPrincipal, + res.mHadCrossOriginRedirects); return res; } diff --git a/dom/canvas/CanvasUtils.cpp b/dom/canvas/CanvasUtils.cpp index 5269c50acc88..dd9271ee59a2 100644 --- a/dom/canvas/CanvasUtils.cpp +++ b/dom/canvas/CanvasUtils.cpp @@ -174,8 +174,7 @@ bool IsImageExtractionAllowed(Document* aDocument, JSContext* aCx, if (XRE_IsContentProcess()) { BrowserChild* browserChild = BrowserChild::GetFrom(win); if (browserChild) { - browserChild->SendShowCanvasPermissionPrompt(origin, - isAutoBlockCanvas); + browserChild->SendShowCanvasPermissionPrompt(origin, isAutoBlockCanvas); } } else { nsCOMPtr obs = mozilla::services::GetObserverService(); @@ -295,12 +294,17 @@ bool HasDrawWindowPrivilege(JSContext* aCx, JSObject* /* unused */) { nsGkAtoms::all_urlsPermission); } -bool CheckWriteOnlySecurity(bool aCORSUsed, nsIPrincipal* aPrincipal) { +bool CheckWriteOnlySecurity(bool aCORSUsed, nsIPrincipal* aPrincipal, + bool aHadCrossOriginRedirects) { if (!aPrincipal) { return true; } if (!aCORSUsed) { + if (aHadCrossOriginRedirects) { + return true; + } + nsIGlobalObject* incumbentSettingsObject = dom::GetIncumbentGlobal(); if (NS_WARN_IF(!incumbentSettingsObject)) { return true; diff --git a/dom/canvas/CanvasUtils.h b/dom/canvas/CanvasUtils.h index c4de43c16879..0da8561d9b34 100644 --- a/dom/canvas/CanvasUtils.h +++ b/dom/canvas/CanvasUtils.h @@ -176,7 +176,8 @@ void DashArrayToJSVal(nsTArray& dashes, JSContext* cx, // returns true if write-only mode must used for this principal based on // the incumbent global. -bool CheckWriteOnlySecurity(bool aCORSUsed, nsIPrincipal* aPrincipal); +bool CheckWriteOnlySecurity(bool aCORSUsed, nsIPrincipal* aPrincipal, + bool aHadCrossOriginRedirects); } // namespace CanvasUtils } // namespace mozilla diff --git a/dom/canvas/ImageBitmap.cpp b/dom/canvas/ImageBitmap.cpp index f5f629c52e95..c4c0fbc8a215 100644 --- a/dom/canvas/ImageBitmap.cpp +++ b/dom/canvas/ImageBitmap.cpp @@ -770,8 +770,10 @@ already_AddRefed ImageBitmap::CreateInternal( // Check security. nsCOMPtr principal = aVideoEl.GetCurrentVideoPrincipal(); + bool hadCrossOriginRedirects = aVideoEl.HadCrossOriginRedirects(); bool CORSUsed = aVideoEl.GetCORSMode() != CORS_NONE; - bool writeOnly = CheckWriteOnlySecurity(CORSUsed, principal); + bool writeOnly = + CheckWriteOnlySecurity(CORSUsed, principal, hadCrossOriginRedirects); // Create ImageBitmap. RefPtr data = aVideoEl.GetCurrentImage(); diff --git a/dom/html/HTMLMediaElement.cpp b/dom/html/HTMLMediaElement.cpp index cb800f24b6a3..d4bc2584870d 100644 --- a/dom/html/HTMLMediaElement.cpp +++ b/dom/html/HTMLMediaElement.cpp @@ -5867,6 +5867,13 @@ already_AddRefed HTMLMediaElement::GetCurrentPrincipal() { return nullptr; } +bool HTMLMediaElement::HadCrossOriginRedirects() { + if (mDecoder) { + return mDecoder->HadCrossOriginRedirects(); + } + return false; +} + already_AddRefed HTMLMediaElement::GetCurrentVideoPrincipal() { if (mDecoder) { return mDecoder->GetCurrentPrincipal(); diff --git a/dom/html/HTMLMediaElement.h b/dom/html/HTMLMediaElement.h index 90a7ac25f976..758e213653c3 100644 --- a/dom/html/HTMLMediaElement.h +++ b/dom/html/HTMLMediaElement.h @@ -279,6 +279,10 @@ class HTMLMediaElement : public nsGenericHTMLElement, // principal. Returns null if nothing is playing. already_AddRefed GetCurrentPrincipal(); + // Return true if the loading of this resource required cross-origin + // redirects. + bool HadCrossOriginRedirects(); + // Principal of the currently playing video resource. Anything accessing the // image container of this element must have a principal that subsumes this // principal. If there are no live video tracks but content has been rendered diff --git a/dom/media/BaseMediaResource.h b/dom/media/BaseMediaResource.h index 69471ff50d1b..1cc6d6b5683a 100644 --- a/dom/media/BaseMediaResource.h +++ b/dom/media/BaseMediaResource.h @@ -72,6 +72,10 @@ class BaseMediaResource : public MediaResource, // Get the current principal for the channel virtual already_AddRefed GetCurrentPrincipal() = 0; + // Return true if the loading of this resource required cross-origin + // redirects. + virtual bool HadCrossOriginRedirects() = 0; + /** * Open the stream. This creates a stream listener and returns it in * aStreamListener; this listener needs to be notified of incoming data. diff --git a/dom/media/ChannelMediaDecoder.cpp b/dom/media/ChannelMediaDecoder.cpp index a391b9663e83..c3c5d6c8ca0a 100644 --- a/dom/media/ChannelMediaDecoder.cpp +++ b/dom/media/ChannelMediaDecoder.cpp @@ -501,6 +501,11 @@ already_AddRefed ChannelMediaDecoder::GetCurrentPrincipal() { return mResource ? mResource->GetCurrentPrincipal() : nullptr; } +bool ChannelMediaDecoder::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + return mResource ? mResource->HadCrossOriginRedirects() : false; +} + bool ChannelMediaDecoder::IsTransportSeekable() { MOZ_ASSERT(NS_IsMainThread()); return mResource->IsTransportSeekable(); diff --git a/dom/media/ChannelMediaDecoder.h b/dom/media/ChannelMediaDecoder.h index 378eb444e91e..3a500890c3cd 100644 --- a/dom/media/ChannelMediaDecoder.h +++ b/dom/media/ChannelMediaDecoder.h @@ -91,6 +91,7 @@ class ChannelMediaDecoder void AddSizeOfResources(ResourceSizes* aSizes) override; already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; bool IsTransportSeekable() override; void SetLoadInBackground(bool aLoadInBackground) override; void Suspend() override; diff --git a/dom/media/ChannelMediaResource.cpp b/dom/media/ChannelMediaResource.cpp index e69a39f89f43..06ab2c0d5011 100644 --- a/dom/media/ChannelMediaResource.cpp +++ b/dom/media/ChannelMediaResource.cpp @@ -11,6 +11,7 @@ #include "nsIClassOfService.h" #include "nsIInputStream.h" #include "nsIThreadRetargetableRequest.h" +#include "nsITimedChannel.h" #include "nsHttp.h" #include "nsNetUtil.h" @@ -604,6 +605,11 @@ already_AddRefed ChannelMediaResource::GetCurrentPrincipal() { return do_AddRef(mSharedInfo->mPrincipal); } +bool ChannelMediaResource::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + return mSharedInfo->mHadCrossOriginRedirects; +} + bool ChannelMediaResource::CanClone() { return !mClosed && mCacheStream.IsAvailableForSharing(); } @@ -816,6 +822,20 @@ void ChannelMediaResource::UpdatePrincipal() { r->CacheClientNotifyPrincipalChanged(); } } + + // ChannelMediaResource can recreate the channel. When this happens, we don't + // want to overwrite mHadCrossOriginRedirects because the new channel could + // skip intermediate redirects. + if (!mSharedInfo->mHadCrossOriginRedirects) { + nsCOMPtr timedChannel = do_QueryInterface(mChannel); + if (timedChannel) { + bool allRedirectsSameOrigin = false; + mSharedInfo->mHadCrossOriginRedirects = + NS_SUCCEEDED(timedChannel->GetAllRedirectsSameOrigin( + &allRedirectsSameOrigin)) && + !allRedirectsSameOrigin; + } + } } void ChannelMediaResource::CacheClientNotifySuspendedStatusChanged( diff --git a/dom/media/ChannelMediaResource.h b/dom/media/ChannelMediaResource.h index 794b0b58f892..a9e07e80691c 100644 --- a/dom/media/ChannelMediaResource.h +++ b/dom/media/ChannelMediaResource.h @@ -66,8 +66,12 @@ class ChannelMediaResource // Store information shared among resources. Main thread only. struct SharedInfo { NS_INLINE_DECL_REFCOUNTING(SharedInfo); + + SharedInfo() : mHadCrossOriginRedirects(false) {} + nsCOMPtr mPrincipal; nsTArray mResources; + bool mHadCrossOriginRedirects; private: ~SharedInfo() = default; @@ -117,6 +121,7 @@ class ChannelMediaResource void Suspend(bool aCloseImmediately) override; void Resume() override; already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; bool CanClone() override; already_AddRefed CloneData( MediaResourceCallback* aDecoder) override; diff --git a/dom/media/CloneableWithRangeMediaResource.cpp b/dom/media/CloneableWithRangeMediaResource.cpp index c3f3e5826811..bf2839ba876e 100644 --- a/dom/media/CloneableWithRangeMediaResource.cpp +++ b/dom/media/CloneableWithRangeMediaResource.cpp @@ -164,6 +164,20 @@ CloneableWithRangeMediaResource::GetCurrentPrincipal() { return principal.forget(); } +bool CloneableWithRangeMediaResource::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + + nsCOMPtr timedChannel = do_QueryInterface(mChannel); + if (!timedChannel) { + return false; + } + + bool allRedirectsSameOrigin = false; + return NS_SUCCEEDED(timedChannel->GetAllRedirectsSameOrigin( + &allRedirectsSameOrigin)) && + !allRedirectsSameOrigin; +} + nsresult CloneableWithRangeMediaResource::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount) { diff --git a/dom/media/CloneableWithRangeMediaResource.h b/dom/media/CloneableWithRangeMediaResource.h index 6793f0d82e88..4f78f798ab44 100644 --- a/dom/media/CloneableWithRangeMediaResource.h +++ b/dom/media/CloneableWithRangeMediaResource.h @@ -31,6 +31,7 @@ class CloneableWithRangeMediaResource : public BaseMediaResource { void Suspend(bool aCloseImmediately) override {} void Resume() override {} already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; nsresult ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount) override; diff --git a/dom/media/FileMediaResource.cpp b/dom/media/FileMediaResource.cpp index 7b427746af85..94671d502178 100644 --- a/dom/media/FileMediaResource.cpp +++ b/dom/media/FileMediaResource.cpp @@ -11,6 +11,7 @@ #include "nsContentUtils.h" #include "nsIFileChannel.h" #include "nsIFileStreams.h" +#include "nsITimedChannel.h" #include "nsNetUtil.h" namespace mozilla { @@ -117,6 +118,20 @@ already_AddRefed FileMediaResource::GetCurrentPrincipal() { return principal.forget(); } +bool FileMediaResource::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + + nsCOMPtr timedChannel = do_QueryInterface(mChannel); + if (!timedChannel) { + return false; + } + + bool allRedirectsSameOrigin = false; + return NS_SUCCEEDED(timedChannel->GetAllRedirectsSameOrigin( + &allRedirectsSameOrigin)) && + !allRedirectsSameOrigin; +} + nsresult FileMediaResource::ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount) { MutexAutoLock lock(mLock); diff --git a/dom/media/FileMediaResource.h b/dom/media/FileMediaResource.h index 7bb143e76cfb..a1a61b56ca1a 100644 --- a/dom/media/FileMediaResource.h +++ b/dom/media/FileMediaResource.h @@ -27,6 +27,7 @@ class FileMediaResource : public BaseMediaResource { void Suspend(bool aCloseImmediately) override {} void Resume() override {} already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; nsresult ReadFromCache(char* aBuffer, int64_t aOffset, uint32_t aCount) override; diff --git a/dom/media/MediaDecoder.h b/dom/media/MediaDecoder.h index b0d3311936a3..92a73367c4f9 100644 --- a/dom/media/MediaDecoder.h +++ b/dom/media/MediaDecoder.h @@ -126,6 +126,10 @@ class MediaDecoder : public DecoderDoctorLifeLogger { // Return the principal of the current URI being played or downloaded. virtual already_AddRefed GetCurrentPrincipal() = 0; + // Return true if the loading of this resource required cross-origin + // redirects. + virtual bool HadCrossOriginRedirects() = 0; + // Return the time position in the video stream being // played measured in seconds. virtual double GetCurrentTime(); diff --git a/dom/media/hls/HLSDecoder.cpp b/dom/media/hls/HLSDecoder.cpp index ed51c4aa27b3..cea80c7d9eab 100644 --- a/dom/media/hls/HLSDecoder.cpp +++ b/dom/media/hls/HLSDecoder.cpp @@ -182,6 +182,12 @@ already_AddRefed HLSDecoder::GetCurrentPrincipal() { return nullptr; } +bool HLSDecoder::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + // Bug 1478843 + return false; +} + void HLSDecoder::Play() { MOZ_ASSERT(NS_IsMainThread()); HLS_DEBUG("HLSDecoder", "MediaElement called Play"); diff --git a/dom/media/hls/HLSDecoder.h b/dom/media/hls/HLSDecoder.h index b0da7932f0df..4eb9dab30e77 100644 --- a/dom/media/hls/HLSDecoder.h +++ b/dom/media/hls/HLSDecoder.h @@ -35,6 +35,7 @@ class HLSDecoder final : public MediaDecoder { void AddSizeOfResources(ResourceSizes* aSizes) override; already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; bool IsTransportSeekable() override { return true; } void Suspend() override; void Resume() override; diff --git a/dom/media/mediasource/MediaSourceDecoder.cpp b/dom/media/mediasource/MediaSourceDecoder.cpp index 4e64db9a4a51..956d91a2e932 100644 --- a/dom/media/mediasource/MediaSourceDecoder.cpp +++ b/dom/media/mediasource/MediaSourceDecoder.cpp @@ -335,6 +335,11 @@ already_AddRefed MediaSourceDecoder::GetCurrentPrincipal() { return do_AddRef(mPrincipal); } +bool MediaSourceDecoder::HadCrossOriginRedirects() { + MOZ_ASSERT(NS_IsMainThread()); + return false; +} + #undef MSE_DEBUG #undef MSE_DEBUGV diff --git a/dom/media/mediasource/MediaSourceDecoder.h b/dom/media/mediasource/MediaSourceDecoder.h index cbd18cbfc4e9..0922f070e037 100644 --- a/dom/media/mediasource/MediaSourceDecoder.h +++ b/dom/media/mediasource/MediaSourceDecoder.h @@ -50,6 +50,8 @@ class MediaSourceDecoder : public MediaDecoder, already_AddRefed GetCurrentPrincipal() override; + bool HadCrossOriginRedirects() override; + bool IsTransportSeekable() override { return true; } // Returns a structure describing the state of the MediaSource internal diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index 01a9f500637d..7452e77c06bb 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/layout/base/nsLayoutUtils.cpp @@ -7514,10 +7514,14 @@ nsLayoutUtils::SurfaceFromElementResult nsLayoutUtils::SurfaceFromElement( result.mCORSUsed = (corsmode != imgIRequest::CORS_NONE); } + bool hadCrossOriginRedirects = true; + imgRequest->GetHadCrossOriginRedirects(&hadCrossOriginRedirects); + result.mPrincipal = principal.forget(); + result.mHadCrossOriginRedirects = hadCrossOriginRedirects; result.mImageRequest = imgRequest.forget(); - result.mIsWriteOnly = - CanvasUtils::CheckWriteOnlySecurity(result.mCORSUsed, result.mPrincipal); + result.mIsWriteOnly = CanvasUtils::CheckWriteOnlySecurity( + result.mCORSUsed, result.mPrincipal, result.mHadCrossOriginRedirects); return result; } @@ -7565,6 +7569,7 @@ nsLayoutUtils::SurfaceFromElementResult nsLayoutUtils::SurfaceFromElement( result.mHasSize = true; result.mSize = size; result.mPrincipal = aElement->NodePrincipal(); + result.mHadCrossOriginRedirects = false; result.mIsWriteOnly = aElement->IsWriteOnly(); return result; @@ -7611,8 +7616,9 @@ nsLayoutUtils::SurfaceFromElementResult nsLayoutUtils::SurfaceFromElement( result.mHasSize = true; result.mSize = result.mLayersImage->GetSize(); result.mPrincipal = principal.forget(); - result.mIsWriteOnly = - CanvasUtils::CheckWriteOnlySecurity(result.mCORSUsed, result.mPrincipal); + result.mHadCrossOriginRedirects = aElement->HadCrossOriginRedirects(); + result.mIsWriteOnly = CanvasUtils::CheckWriteOnlySecurity( + result.mCORSUsed, result.mPrincipal, result.mHadCrossOriginRedirects); return result; } @@ -8626,7 +8632,8 @@ bool nsLayoutUtils::IsAPZTestLoggingEnabled() { nsLayoutUtils::SurfaceFromElementResult::SurfaceFromElementResult() // Use safe default values here - : mIsWriteOnly(true), + : mHadCrossOriginRedirects(false), + mIsWriteOnly(true), mIsStillLoading(false), mHasSize(false), mCORSUsed(false), diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index cc78b6c6d72d..daed51f139fa 100644 --- a/layout/base/nsLayoutUtils.h +++ b/layout/base/nsLayoutUtils.h @@ -2163,6 +2163,9 @@ class nsLayoutUtils { nsCOMPtr mPrincipal; /* The image request, if the element is an nsIImageLoadingContent */ nsCOMPtr mImageRequest; + /* True if cross-origins redirects have been done in order to load this + * resource */ + bool mHadCrossOriginRedirects; /* Whether the element was "write only", that is, the bits should not be * exposed to content */ bool mIsWriteOnly;