From 84c462a2d06f61d73893d2b44b79aa8f5bcb700a Mon Sep 17 00:00:00 2001 From: Jonathan Kingston Date: Mon, 13 Jan 2020 11:47:25 +0000 Subject: [PATCH] Bug 1605814 - Don't capture CSP nonce when loading a child sheet. r=emilio,ckerschb Differential Revision: https://phabricator.services.mozilla.com/D58686 --HG-- extra : moz-landing-system : lando --- dom/base/nsContentSink.cpp | 2 ++ dom/base/nsIStyleSheetLinkingElement.h | 6 +++++- dom/base/nsStyleLinkElement.cpp | 12 ++++++------ dom/html/HTMLLinkElement.cpp | 9 +++++++++ dom/html/HTMLStyleElement.cpp | 8 ++++++-- dom/svg/SVGStyleElement.cpp | 4 ++++ dom/xml/XMLStylesheetProcessingInstruction.cpp | 2 ++ layout/style/Loader.cpp | 17 ++++++++--------- layout/style/Loader.h | 2 +- 9 files changed, 43 insertions(+), 19 deletions(-) diff --git a/dom/base/nsContentSink.cpp b/dom/base/nsContentSink.cpp index 0d8b71b58dd0..25938965bf8c 100644 --- a/dom/base/nsContentSink.cpp +++ b/dom/base/nsContentSink.cpp @@ -681,6 +681,8 @@ nsresult nsContentSink::ProcessStyleLinkFromHeader( CORS_NONE, aTitle, aMedia, + /* integrity = */ EmptyString(), + /* nonce = */ EmptyString(), aAlternate ? Loader::HasAlternateRel::Yes : Loader::HasAlternateRel::No, Loader::IsInline::No, Loader::IsExplicitlyEnabled::No, diff --git a/dom/base/nsIStyleSheetLinkingElement.h b/dom/base/nsIStyleSheetLinkingElement.h index 191eef565306..0deceab8af5d 100644 --- a/dom/base/nsIStyleSheetLinkingElement.h +++ b/dom/base/nsIStyleSheetLinkingElement.h @@ -98,6 +98,7 @@ class nsIStyleSheetLinkingElement : public nsISupports { nsString mTitle; nsString mMedia; nsString mIntegrity; + nsString mNonce; bool mHasAlternateRel; bool mIsInline; @@ -108,7 +109,10 @@ class nsIStyleSheetLinkingElement : public nsISupports { already_AddRefed aTriggeringPrincipal, already_AddRefed aReferrerInfo, mozilla::CORSMode, const nsAString& aTitle, - const nsAString& aMedia, HasAlternateRel, IsInline, + const nsAString& aMedia, + const nsAString& aIntegrity, + const nsAString& aNonce, + HasAlternateRel, IsInline, IsExplicitlyEnabled); ~SheetInfo(); diff --git a/dom/base/nsStyleLinkElement.cpp b/dom/base/nsStyleLinkElement.cpp index a4e4f1742cd4..2e5bae0da621 100644 --- a/dom/base/nsStyleLinkElement.cpp +++ b/dom/base/nsStyleLinkElement.cpp @@ -40,7 +40,8 @@ nsStyleLinkElement::SheetInfo::SheetInfo( already_AddRefed aTriggeringPrincipal, already_AddRefed aReferrerInfo, mozilla::CORSMode aCORSMode, const nsAString& aTitle, - const nsAString& aMedia, HasAlternateRel aHasAlternateRel, + const nsAString& aMedia, const nsAString& aIntegrity, + const nsAString& aNonce, HasAlternateRel aHasAlternateRel, IsInline aIsInline, IsExplicitlyEnabled aIsExplicitlyEnabled) : mContent(aContent), mURI(aURI), @@ -49,17 +50,16 @@ nsStyleLinkElement::SheetInfo::SheetInfo( mCORSMode(aCORSMode), mTitle(aTitle), mMedia(aMedia), + mIntegrity(aIntegrity), + mNonce(aNonce), mHasAlternateRel(aHasAlternateRel == HasAlternateRel::Yes), mIsInline(aIsInline == IsInline::Yes), mIsExplicitlyEnabled(aIsExplicitlyEnabled) { MOZ_ASSERT(!mIsInline || aContent); MOZ_ASSERT_IF(aContent, aContent->OwnerDoc() == &aDocument); MOZ_ASSERT(mReferrerInfo); - - if (!mIsInline && aContent && aContent->IsElement()) { - aContent->AsElement()->GetAttr(kNameSpaceID_None, nsGkAtoms::integrity, - mIntegrity); - } + MOZ_ASSERT(mIntegrity.IsEmpty() || !mIsInline, + "Integrity only applies to "); } nsStyleLinkElement::SheetInfo::~SheetInfo() = default; diff --git a/dom/html/HTMLLinkElement.cpp b/dom/html/HTMLLinkElement.cpp index 218fd820656c..6968f7f58854 100644 --- a/dom/html/HTMLLinkElement.cpp +++ b/dom/html/HTMLLinkElement.cpp @@ -464,10 +464,17 @@ Maybe HTMLLinkElement::GetStyleSheetInfo() { return Nothing(); } + nsAutoString integrity; + GetAttr(kNameSpaceID_None, nsGkAtoms::integrity, integrity); + nsCOMPtr uri = Link::GetURI(); nsCOMPtr prin = mTriggeringPrincipal; nsCOMPtr referrerInfo = new ReferrerInfo(); referrerInfo->InitWithNode(this); + + nsAutoString nonce; + GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce); + return Some(SheetInfo{ *OwnerDoc(), this, @@ -477,6 +484,8 @@ Maybe HTMLLinkElement::GetStyleSheetInfo() { GetCORSMode(), title, media, + integrity, + nonce, alternate ? HasAlternateRel::Yes : HasAlternateRel::No, IsInline::No, mExplicitlyEnabled ? IsExplicitlyEnabled::Yes : IsExplicitlyEnabled::No, diff --git a/dom/html/HTMLStyleElement.cpp b/dom/html/HTMLStyleElement.cpp index f67dff5a6b3f..45d5744b7c46 100644 --- a/dom/html/HTMLStyleElement.cpp +++ b/dom/html/HTMLStyleElement.cpp @@ -174,16 +174,20 @@ Maybe HTMLStyleElement::GetStyleSheetInfo() { nsCOMPtr referrerInfo = new ReferrerInfo(); referrerInfo->InitWithNode(this); - nsCOMPtr prin = mTriggeringPrincipal; + return Some(SheetInfo{ *OwnerDoc(), this, nullptr, - prin.forget(), + do_AddRef(mTriggeringPrincipal), referrerInfo.forget(), CORS_NONE, title, media, + /* integrity = */ EmptyString(), + /* nsStyleUtil::CSPAllowsInlineStyle takes care of nonce checking for + inline styles. Bug 1607011 */ + /* nonce = */ EmptyString(), HasAlternateRel::No, IsInline::Yes, IsExplicitlyEnabled::No, diff --git a/dom/svg/SVGStyleElement.cpp b/dom/svg/SVGStyleElement.cpp index 03554f135b02..5d97a89a76b0 100644 --- a/dom/svg/SVGStyleElement.cpp +++ b/dom/svg/SVGStyleElement.cpp @@ -195,6 +195,10 @@ Maybe SVGStyleElement::GetStyleSheetInfo() { AttrValueToCORSMode(GetParsedAttr(nsGkAtoms::crossorigin)), title, media, + /* integrity = */ EmptyString(), + /* nsStyleUtil::CSPAllowsInlineStyle takes care of nonce checking for + inline styles. Bug 1607011 */ + /* nonce = */ EmptyString(), HasAlternateRel::No, IsInline::Yes, IsExplicitlyEnabled::No, diff --git a/dom/xml/XMLStylesheetProcessingInstruction.cpp b/dom/xml/XMLStylesheetProcessingInstruction.cpp index 0b83fa8b575a..4c472835ad04 100644 --- a/dom/xml/XMLStylesheetProcessingInstruction.cpp +++ b/dom/xml/XMLStylesheetProcessingInstruction.cpp @@ -136,6 +136,8 @@ XMLStylesheetProcessingInstruction::GetStyleSheetInfo() { CORS_NONE, title, media, + /* integrity = */ EmptyString(), + /* nonce = */ EmptyString(), alternate ? HasAlternateRel::Yes : HasAlternateRel::No, IsInline::No, IsExplicitlyEnabled::No, diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 83cbf99ef049..53a0c1bfee01 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -1066,6 +1066,7 @@ nsresult Loader::CheckContentPolicy(nsIPrincipal* aLoadingPrincipal, nsIPrincipal* aTriggeringPrincipal, nsIURI* aTargetURI, nsINode* aRequestingNode, + const nsAString& aNonce, IsPreload aIsPreload) { // When performing a system load (e.g. aUseSystemPrincipal = true) // then aLoadingPrincipal == null; don't consult content policies. @@ -1084,12 +1085,8 @@ nsresult Loader::CheckContentPolicy(nsIPrincipal* aLoadingPrincipal, // snapshot the nonce at load start time for performing CSP checks if (contentPolicyType == nsIContentPolicy::TYPE_INTERNAL_STYLESHEET) { - nsCOMPtr element = do_QueryInterface(aRequestingNode); - if (element && element->IsHTMLElement()) { - nsAutoString cspNonce; - element->GetAttr(nsGkAtoms::nonce, cspNonce); - secCheckLoadInfo->SetCspNonce(cspNonce); - } + secCheckLoadInfo->SetCspNonce(aNonce); + MOZ_ASSERT_IF(aIsPreload != IsPreload::No, aNonce.IsEmpty()); } int16_t shouldLoad = nsIContentPolicy::ACCEPT; @@ -1404,6 +1401,7 @@ nsresult Loader::LoadSheet(SheetLoadData& aLoadData, SheetState aSheetState, nsCOMPtr element = do_QueryInterface(aLoadData.mRequestingNode); if (element && element->IsHTMLElement()) { nsAutoString cspNonce; + // TODO(bug 1607009) move to SheetLoadData element->GetAttr(nsGkAtoms::nonce, cspNonce); nsCOMPtr loadInfo = channel->LoadInfo(); loadInfo->SetCspNonce(cspNonce); @@ -1534,6 +1532,7 @@ nsresult Loader::LoadSheet(SheetLoadData& aLoadData, SheetState aSheetState, nsCOMPtr element = do_QueryInterface(aLoadData.mRequestingNode); if (element && element->IsHTMLElement()) { nsAutoString cspNonce; + // TODO(bug 1607009) move to SheetLoadData element->GetAttr(nsGkAtoms::nonce, cspNonce); nsCOMPtr loadInfo = channel->LoadInfo(); loadInfo->SetCspNonce(cspNonce); @@ -2018,7 +2017,7 @@ Result Loader::LoadStyleLink( MOZ_ASSERT_IF(syncLoad, !aObserver); nsresult rv = CheckContentPolicy(loadingPrincipal, principal, aInfo.mURI, - context, IsPreload::No); + context, aInfo.mNonce, IsPreload::No); if (NS_WARN_IF(NS_FAILED(rv))) { // Don't fire the error event if our document is loaded as data. We're // supposed to not even try to do loads in that case... Unfortunately, we @@ -2167,7 +2166,7 @@ nsresult Loader::LoadChildSheet(StyleSheet& aParentSheet, nsIPrincipal* principal = aParentSheet.Principal(); nsresult rv = CheckContentPolicy(loadingPrincipal, principal, aURL, context, - IsPreload::No); + EmptyString(), IsPreload::No); if (NS_WARN_IF(NS_FAILED(rv))) { if (aParentData) { MarkLoadTreeFailed(*aParentData); @@ -2292,7 +2291,7 @@ Result, nsresult> Loader::InternalLoadNonDocumentSheet( nsCOMPtr loadingPrincipal = (aOriginPrincipal && mDocument ? mDocument->NodePrincipal() : nullptr); nsresult rv = CheckContentPolicy(loadingPrincipal, aOriginPrincipal, aURL, - mDocument, aIsPreload); + mDocument, EmptyString(), aIsPreload); if (NS_FAILED(rv)) { return Err(rv); } diff --git a/layout/style/Loader.h b/layout/style/Loader.h index 68a78a9cc937..4a0e51065176 100644 --- a/layout/style/Loader.h +++ b/layout/style/Loader.h @@ -331,7 +331,7 @@ class Loader final { nsresult CheckContentPolicy(nsIPrincipal* aLoadingPrincipal, nsIPrincipal* aTriggeringPrincipal, nsIURI* aTargetURI, nsINode* aRequestingNode, - IsPreload); + const nsAString& aNonce, IsPreload); enum class SheetState : uint8_t { Unknown = 0,