From bfabbcd311405ef312387b1093c72b93e006b742 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Wed, 3 Jan 2024 15:34:22 -0600 Subject: [PATCH 01/25] Issue #2402 - Ignore empty CSP directives. https://bugzilla.mozilla.org/show_bug.cgi?id=1439425 --- dom/security/nsCSPParser.cpp | 4 ++++ dom/security/nsCSPUtils.cpp | 7 +++++++ dom/security/nsCSPUtils.h | 1 + 3 files changed, 12 insertions(+) diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp index 1012efe878..36505877ae 100644 --- a/dom/security/nsCSPParser.cpp +++ b/dom/security/nsCSPParser.cpp @@ -1115,6 +1115,10 @@ nsCSPParser::directive() return; } + if (CSP_IsEmptyDirective(mCurValue, mCurToken)) { + return; + } + // Try to create a new CSPDirective nsCSPDirective* cspDir = directiveName(); if (!cspDir) { diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index 9459c65cf0..b73bea30a5 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/dom/security/nsCSPUtils.cpp @@ -292,6 +292,13 @@ CSP_CreateHostSrcFromSelfURI(nsIURI* aSelfURI) return hostsrc; } +bool +CSP_IsEmptyDirective(const nsAString& aValue, const nsAString& aDir) +{ + return (aDir.Length() == 0 && + aValue.Length() == 0); +} + bool CSP_IsValidDirective(const nsAString& aDir) { diff --git a/dom/security/nsCSPUtils.h b/dom/security/nsCSPUtils.h index b06f9d3c84..b4c48fb42d 100644 --- a/dom/security/nsCSPUtils.h +++ b/dom/security/nsCSPUtils.h @@ -235,6 +235,7 @@ nsresult CSP_AppendCSPFromHeader(nsIContentSecurityPolicy* aCsp, class nsCSPHostSrc; nsCSPHostSrc* CSP_CreateHostSrcFromSelfURI(nsIURI* aSelfURI); +bool CSP_IsEmptyDirective(const nsAString& aValue, const nsAString& aDir); bool CSP_IsValidDirective(const nsAString& aDir); bool CSP_IsDirective(const nsAString& aValue, CSPDirective aDir); bool CSP_IsKeyword(const nsAString& aValue, enum CSPKeyword aKey); From 1349cf2d013a252899984697a4a59a8056cc64f2 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 4 Jan 2024 03:04:31 -0600 Subject: [PATCH 02/25] Issue #2402 - Remove child-src deprecation warning. https://bugzilla.mozilla.org/show_bug.cgi?id=1486331 --- dom/locales/en-US/chrome/security/csp.properties | 4 ---- dom/security/nsCSPParser.cpp | 10 +++------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/dom/locales/en-US/chrome/security/csp.properties b/dom/locales/en-US/chrome/security/csp.properties index 4c4054cee8..da38227403 100644 --- a/dom/locales/en-US/chrome/security/csp.properties +++ b/dom/locales/en-US/chrome/security/csp.properties @@ -112,10 +112,6 @@ couldntParsePort = Couldn’t parse port in %1$S # LOCALIZATION NOTE (duplicateDirective): # %1$S is the name of the duplicate directive duplicateDirective = Duplicate %1$S directives detected. All but the first instance will be ignored. -# LOCALIZATION NOTE (deprecatedChildSrcDirective): -# %1$S is the value of the deprecated directive. -# Do not localize: worker-src, frame-src -deprecatedChildSrcDirective = Directive ‘%1$S’ has been deprecated. Please use directive ‘worker-src’ to control workers, or directive ‘frame-src’ to control frames respectively. # LOCALIZATION NOTE (couldntParseInvalidSandboxFlag): # %1$S is the option that could not be understood couldntParseInvalidSandboxFlag = Couldn’t parse invalid sandbox flag ‘%1$S’ diff --git a/dom/security/nsCSPParser.cpp b/dom/security/nsCSPParser.cpp index 36505877ae..1ec8194513 100644 --- a/dom/security/nsCSPParser.cpp +++ b/dom/security/nsCSPParser.cpp @@ -1056,14 +1056,10 @@ nsCSPParser::directiveName() return new nsUpgradeInsecureDirective(CSP_StringToCSPDirective(mCurToken)); } - // child-src by itself is deprecatd but will be enforced - // * for workers (if worker-src is not explicitly specified) - // * for frames (if frame-src is not explicitly specified) + // if we have a child-src, cache it as a fallback for + // * workers (if worker-src is not explicitly specified) + // * frames (if frame-src is not explicitly specified) if (CSP_IsDirective(mCurToken, nsIContentSecurityPolicy::CHILD_SRC_DIRECTIVE)) { - const char16_t* params[] = { mCurToken.get() }; - logWarningErrorToConsole(nsIScriptError::warningFlag, - "deprecatedChildSrcDirective", - params, ArrayLength(params)); mChildSrc = new nsCSPChildSrcDirective(CSP_StringToCSPDirective(mCurToken)); return mChildSrc; } From ec29404083e6c87725ad290b3a30479106bec76b Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 4 Jan 2024 04:43:00 -0600 Subject: [PATCH 03/25] Issue #2402 - Implement security policy violation event. https://bugzilla.mozilla.org/show_bug.cgi?id=1037335 --- .../security/nsIContentSecurityPolicy.idl | 2 - dom/security/nsCSPContext.cpp | 187 +++++++++++++----- dom/security/nsCSPContext.h | 44 ++++- .../SecurityPolicyViolationEvent.webidl | 41 ++++ dom/webidl/moz.build | 1 + 5 files changed, 215 insertions(+), 60 deletions(-) create mode 100644 dom/webidl/SecurityPolicyViolationEvent.webidl diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl index e76c39c44b..a8b508ce7e 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl @@ -6,11 +6,9 @@ #include "nsIContentPolicy.idl" interface nsIURI; -interface nsIChannel; interface nsIDocShell; interface nsIDOMDocument; interface nsIPrincipal; -interface nsIURI; /** * nsIContentSecurityPolicy diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 8cc83a6d97..48245a1f7c 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -799,31 +799,16 @@ StripURIForReporting(nsIURI* aURI, aURI->GetSpecIgnoringRef(outStrippedURI); } -/** - * Sends CSP violation reports to all sources listed under report-uri. - * - * @param aBlockedContentSource - * Either a CSP Source (like 'self', as string) or nsIURI: the source - * of the violation. - * @param aOriginalUri - * The original URI if the blocked content is a redirect, else null - * @param aViolatedDirective - * the directive that was violated (string). - * @param aSourceFile - * name of the file containing the inline script violation - * @param aScriptSample - * a sample of the violating inline script - * @param aLineNum - * source line number of the violation (if available) - */ nsresult -nsCSPContext::SendReports(nsISupports* aBlockedContentSource, - nsIURI* aOriginalURI, - nsAString& aViolatedDirective, - uint32_t aViolatedPolicyIndex, - nsAString& aSourceFile, - nsAString& aScriptSample, - uint32_t aLineNum) +nsCSPContext::GatherSecurityPolicyViolationEventData( + nsISupports* aBlockedContentSource, + nsIURI* aOriginalURI, + nsAString& aViolatedDirective, + uint32_t aViolatedPolicyIndex, + nsAString& aSourceFile, + nsAString& aScriptSample, + uint32_t aLineNum, + mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) { NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1); @@ -837,9 +822,16 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, return NS_OK; } - dom::CSPReport report; nsresult rv; + // document-uri + nsAutoCString reportDocumentURI; + StripURIForReporting(mSelfURI, mSelfURI, reportDocumentURI); + aViolationEventInit.mDocumentURI = NS_ConvertUTF8toUTF16(reportDocumentURI); + + // referrer + aViolationEventInit.mReferrer = mReferrer; + // blocked-uri if (aBlockedContentSource) { nsAutoCString reportBlockedURI; @@ -858,27 +850,20 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, // ancestor is cross-origin. NS_WARNING("No blocked URI (null aBlockedContentSource) for CSP violation report."); } - report.mCsp_report.mBlocked_uri = NS_ConvertUTF8toUTF16(reportBlockedURI); + aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI); } - // document-uri - nsAutoCString reportDocumentURI; - StripURIForReporting(mSelfURI, mSelfURI, reportDocumentURI); - report.mCsp_report.mDocument_uri = NS_ConvertUTF8toUTF16(reportDocumentURI); + // violated-directive + aViolationEventInit.mViolatedDirective = aViolatedDirective; + + // effective-directive + aViolationEventInit.mEffectiveDirective = aViolatedDirective; // original-policy nsAutoString originalPolicy; rv = this->GetPolicyString(aViolatedPolicyIndex, originalPolicy); NS_ENSURE_SUCCESS(rv, rv); - report.mCsp_report.mOriginal_policy = originalPolicy; - - // referrer - if (!mReferrer.IsEmpty()) { - report.mCsp_report.mReferrer = mReferrer; - } - - // violated-directive - report.mCsp_report.mViolated_directive = aViolatedDirective; + aViolationEventInit.mOriginalPolicy = originalPolicy; // source-file if (!aSourceFile.IsEmpty()) { @@ -890,20 +875,87 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, sourceURI->GetSpecIgnoringRef(spec); aSourceFile = NS_ConvertUTF8toUTF16(spec); } + aViolationEventInit.mSourceFile = aSourceFile; + } + + // sample + aViolationEventInit.mSample = aScriptSample; + + // disposition + aViolationEventInit.mDisposition = mPolicies[aViolatedPolicyIndex]->getReportOnlyFlag() + ? mozilla::dom::SecurityPolicyViolationEventDisposition::Report + : mozilla::dom::SecurityPolicyViolationEventDisposition::Enforce; + + // status-code + uint16_t statusCode = 0; + { + nsCOMPtr doc = do_QueryReferent(mLoadingContext); + if (doc) { + nsCOMPtr channel = do_QueryInterface(doc->GetChannel()); + if (channel) { + uint32_t responseStatus = 0; + nsresult rv = channel->GetResponseStatus(&responseStatus); + if (NS_SUCCEEDED(rv) && (responseStatus <= UINT16_MAX)) { + statusCode = static_cast(responseStatus); + } + } + } + } + aViolationEventInit.mStatusCode = statusCode; + + // line-number + aViolationEventInit.mLineNumber = aLineNum; + + // column-number + // TODO: Set correct column number. + aViolationEventInit.mColumnNumber = 0; + + aViolationEventInit.mBubbles = true; + aViolationEventInit.mComposed = true; + + return NS_OK; +} + +nsresult +nsCSPContext::SendReports( + const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit, + uint32_t aViolatedPolicyIndex) +{ + NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1); + + dom::CSPReport report; + + // blocked-uri + report.mCsp_report.mBlocked_uri = aViolationEventInit.mBlockedURI; + + // document-uri + report.mCsp_report.mDocument_uri = aViolationEventInit.mDocumentURI; + + // original-policy + report.mCsp_report.mOriginal_policy = aViolationEventInit.mOriginalPolicy; + + // referrer + report.mCsp_report.mReferrer = aViolationEventInit.mReferrer; + + // violated-directive + report.mCsp_report.mViolated_directive = aViolationEventInit.mViolatedDirective; + + // source-file + if (!aViolationEventInit.mSourceFile.IsEmpty()) { report.mCsp_report.mSource_file.Construct(); - report.mCsp_report.mSource_file.Value() = aSourceFile; + report.mCsp_report.mSource_file.Value() = aViolationEventInit.mSourceFile; } // script-sample - if (!aScriptSample.IsEmpty()) { + if (!aViolationEventInit.mSample.IsEmpty()) { report.mCsp_report.mScript_sample.Construct(); - report.mCsp_report.mScript_sample.Value() = aScriptSample; + report.mCsp_report.mScript_sample.Value() = aViolationEventInit.mSample; } // line-number - if (aLineNum != 0) { + if (aViolationEventInit.mLineNumber != 0) { report.mCsp_report.mLine_number.Construct(); - report.mCsp_report.mLine_number.Value() = aLineNum; + report.mCsp_report.mLine_number.Value() = aViolationEventInit.mLineNumber; } nsString csp_report; @@ -916,11 +968,11 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, nsTArray reportURIs; mPolicies[aViolatedPolicyIndex]->getReportURIs(reportURIs); - nsCOMPtr doc = do_QueryReferent(mLoadingContext); nsCOMPtr reportURI; nsCOMPtr reportChannel; + nsresult rv; for (uint32_t r = 0; r < reportURIs.Length(); r++) { nsAutoCString reportURICstring = NS_ConvertUTF16toUTF8(reportURIs[r]); // try to create a new uri from every report-uri string @@ -930,7 +982,8 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, CSPCONTEXTLOG(("Could not create nsIURI for report URI %s", reportURICstring.get())); logToConsole(u"triedToSendReport", params, ArrayLength(params), - aSourceFile, aScriptSample, aLineNum, 0, nsIScriptError::errorFlag); + aViolationEventInit.mSourceFile, aViolationEventInit.mSample, + aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); continue; // don't return yet, there may be more URIs } @@ -971,7 +1024,8 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, if (!isHttpScheme) { const char16_t* params[] = { reportURIs[r].get() }; logToConsole(u"reportURInotHttpsOrHttp2", params, ArrayLength(params), - aSourceFile, aScriptSample, aLineNum, 0, nsIScriptError::errorFlag); + aViolationEventInit.mSourceFile, aViolationEventInit.mSample, + aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); continue; } @@ -1035,7 +1089,8 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, const char16_t* params[] = { reportURIs[r].get() }; CSPCONTEXTLOG(("AsyncOpen failed for report URI %s", params[0])); logToConsole(u"triedToSendReport", params, ArrayLength(params), - aSourceFile, aScriptSample, aLineNum, 0, nsIScriptError::errorFlag); + aViolationEventInit.mSourceFile, aViolationEventInit.mSample, + aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); } else { CSPCONTEXTLOG(("Sent violation report to URI %s", reportURICstring.get())); } @@ -1043,6 +1098,26 @@ nsCSPContext::SendReports(nsISupports* aBlockedContentSource, return NS_OK; } +nsresult +nsCSPContext::FireViolationEvent( + const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) +{ + nsCOMPtr doc = do_QueryReferent(mLoadingContext); + if (!doc) { + return NS_OK; + } + + RefPtr event = + mozilla::dom::SecurityPolicyViolationEvent::Constructor( + doc, + NS_LITERAL_STRING("securitypolicyviolation"), + aViolationEventInit); + event->SetTrusted(true); + + bool rv; + return doc->DispatchEvent(event, &rv); +} + /** * Dispatched from the main thread to send reports for one CSP violation. */ @@ -1088,6 +1163,14 @@ class CSPReportSenderRunnable final : public Runnable { MOZ_ASSERT(NS_IsMainThread()); + // 0) prepare violation data + mozilla::dom::SecurityPolicyViolationEventInit init; + mCSPContext->GatherSecurityPolicyViolationEventData( + mBlockedContentSource, mOriginalURI, + mViolatedDirective, mViolatedPolicyIndex, + mSourceFile, mScriptSample, mLineNum, + init); + // 1) notify observers nsCOMPtr observerService = mozilla::services::GetObserverService(); NS_ASSERTION(observerService, "needs observer service"); @@ -1097,9 +1180,7 @@ class CSPReportSenderRunnable final : public Runnable NS_ENSURE_SUCCESS(rv, rv); // 2) send reports for the policy that was violated - mCSPContext->SendReports(mBlockedContentSource, mOriginalURI, - mViolatedDirective, mViolatedPolicyIndex, - mSourceFile, mScriptSample, mLineNum); + mCSPContext->SendReports(init, mViolatedPolicyIndex); // 3) log to console (one per policy violation) // mBlockedContentSource could be a URI or a string. @@ -1130,6 +1211,10 @@ class CSPReportSenderRunnable final : public Runnable params, ArrayLength(params), mSourceFile, mScriptSample, mLineNum, 0, nsIScriptError::errorFlag); } + + // 4) fire violation event + mCSPContext->FireViolationEvent(init); + return NS_OK; } diff --git a/dom/security/nsCSPContext.h b/dom/security/nsCSPContext.h index 3530c74c5c..8a283c1a10 100644 --- a/dom/security/nsCSPContext.h +++ b/dom/security/nsCSPContext.h @@ -7,6 +7,7 @@ #define nsCSPContext_h___ #include "mozilla/dom/nsCSPUtils.h" +#include "mozilla/dom/SecurityPolicyViolationEvent.h" #include "nsDataHashtable.h" #include "nsIChannel.h" #include "nsIChannelEventSink.h" @@ -56,13 +57,42 @@ class nsCSPContext : public nsIContentSecurityPolicy uint32_t aColumnNumber, uint32_t aSeverityFlag); - nsresult SendReports(nsISupports* aBlockedContentSource, - nsIURI* aOriginalURI, - nsAString& aViolatedDirective, - uint32_t aViolatedPolicyIndex, - nsAString& aSourceFile, - nsAString& aScriptSample, - uint32_t aLineNum); + + /** + * Construct SecurityPolicyViolationEventInit structure. + * + * @param aBlockedContentSource + * Either a CSP Source (like 'self', as string) or nsIURI: the source + * of the violation. + * @param aOriginalUri + * The original URI if the blocked content is a redirect, else null + * @param aViolatedDirective + * the directive that was violated (string). + * @param aSourceFile + * name of the file containing the inline script violation + * @param aScriptSample + * a sample of the violating inline script + * @param aLineNum + * source line number of the violation (if available) + * @param aViolationEventInit + * The output + */ + nsresult GatherSecurityPolicyViolationEventData( + nsISupports* aBlockedContentSource, + nsIURI* aOriginalURI, + nsAString& aViolatedDirective, + uint32_t aViolatedPolicyIndex, + nsAString& aSourceFile, + nsAString& aScriptSample, + uint32_t aLineNum, + mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit); + + nsresult SendReports( + const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit, + uint32_t aViolatedPolicyIndex); + + nsresult FireViolationEvent( + const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit); nsresult AsyncReportViolation(nsISupports* aBlockedContentSource, nsIURI* aOriginalURI, diff --git a/dom/webidl/SecurityPolicyViolationEvent.webidl b/dom/webidl/SecurityPolicyViolationEvent.webidl new file mode 100644 index 0000000000..0f3d5db09e --- /dev/null +++ b/dom/webidl/SecurityPolicyViolationEvent.webidl @@ -0,0 +1,41 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +enum SecurityPolicyViolationEventDisposition +{ + "enforce", "report" +}; + +[Constructor(DOMString type, optional SecurityPolicyViolationEventInit eventInitDict)] +interface SecurityPolicyViolationEvent : Event +{ + readonly attribute DOMString documentURI; + readonly attribute DOMString referrer; + readonly attribute DOMString blockedURI; + readonly attribute DOMString violatedDirective; + readonly attribute DOMString effectiveDirective; + readonly attribute DOMString originalPolicy; + readonly attribute DOMString sourceFile; + readonly attribute DOMString sample; + readonly attribute SecurityPolicyViolationEventDisposition disposition; + readonly attribute unsigned short statusCode; + readonly attribute long lineNumber; + readonly attribute long columnNumber; +}; + +dictionary SecurityPolicyViolationEventInit : EventInit +{ + DOMString documentURI = ""; + DOMString referrer = ""; + DOMString blockedURI = ""; + DOMString violatedDirective = ""; + DOMString effectiveDirective = ""; + DOMString originalPolicy = ""; + DOMString sourceFile = ""; + DOMString sample = ""; + SecurityPolicyViolationEventDisposition disposition = "report"; + unsigned short statusCode = 0; + long lineNumber = 0; + long columnNumber = 0; +}; \ No newline at end of file diff --git a/dom/webidl/moz.build b/dom/webidl/moz.build index c1f5ad1b76..e7cc11cecc 100644 --- a/dom/webidl/moz.build +++ b/dom/webidl/moz.build @@ -698,6 +698,7 @@ GENERATED_EVENTS_WEBIDL_FILES = [ 'PromiseRejectionEvent.webidl', 'RecordErrorEvent.webidl', 'ScrollViewChangeEvent.webidl', + 'SecurityPolicyViolationEvent.webidl', 'ServiceWorkerMessageEvent.webidl', 'StyleRuleChangeEvent.webidl', 'StyleSheetApplicableStateChangeEvent.webidl', From 1e0851158e69f5d1d587ddea060da714861aa000 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 4 Jan 2024 05:04:21 -0600 Subject: [PATCH 04/25] Issue #2402 - Fix SecurityPolicyViolationEvent.violatedDirective. https://bugzilla.mozilla.org/show_bug.cgi?id=1418243 --- dom/security/nsCSPContext.cpp | 50 +++++++++++++++++++++++++++++------ dom/security/nsCSPUtils.cpp | 35 +++++++++++++++++++++--- dom/security/nsCSPUtils.h | 7 +++++ 3 files changed, 80 insertions(+), 12 deletions(-) diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 48245a1f7c..3b74aea09c 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -3,6 +3,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +#include +#include + #include "nsCOMPtr.h" #include "nsContentPolicyUtils.h" #include "nsContentUtils.h" @@ -58,6 +61,29 @@ GetCspContextLog() static const uint32_t CSP_CACHE_URI_CUTOFF_SIZE = 512; +#ifdef DEBUG +/** + * This function is only used for verification purposes within + * GatherSecurityPolicyViolationEventData. + */ +static bool +ValidateDirectiveName(const nsAString& aDirective) +{ + static const auto directives = [] () { + std::unordered_set directives; + constexpr size_t dirLen = sizeof(CSPStrDirectives) / sizeof(CSPStrDirectives[0]); + for (size_t i = 0; i < dirLen; ++i) { + directives.insert(CSPStrDirectives[i]); + } + return directives; + } (); + + nsAutoString directive(aDirective); + auto itr = directives.find(NS_ConvertUTF16toUTF8(directive).get()); + return itr != directives.end(); +} +#endif // DEBUG + /** * Creates a key for use in the ShouldLoad cache. * Looks like: ! @@ -812,6 +838,8 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( { NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1); + MOZ_ASSERT(ValidateDirectiveName(aViolatedDirective), "Invalid directive name"); + if (!CSPService::sCSPReportingEnabled) { // Reporting is pref-disabled. Don't do any actual work and return success. nsContentUtils::ReportToConsoleNonLocalized( @@ -853,12 +881,15 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI); } - // violated-directive - aViolationEventInit.mViolatedDirective = aViolatedDirective; - // effective-directive + // The name of the policy directive that was violated. aViolationEventInit.mEffectiveDirective = aViolatedDirective; + // violated-directive + // In CSP2, the policy directive that was violated, as it appears in the policy. + // In CSP3, the same as effective-directive. + aViolationEventInit.mViolatedDirective = aViolatedDirective; + // original-policy nsAutoString originalPolicy; rv = this->GetPolicyString(aViolatedPolicyIndex, originalPolicy); @@ -1163,20 +1194,23 @@ class CSPReportSenderRunnable final : public Runnable { MOZ_ASSERT(NS_IsMainThread()); + nsresult rv; + // 0) prepare violation data mozilla::dom::SecurityPolicyViolationEventInit init; - mCSPContext->GatherSecurityPolicyViolationEventData( + rv = mCSPContext->GatherSecurityPolicyViolationEventData( mBlockedContentSource, mOriginalURI, mViolatedDirective, mViolatedPolicyIndex, mSourceFile, mScriptSample, mLineNum, init); - + NS_ENSURE_SUCCESS(rv, rv); + // 1) notify observers nsCOMPtr observerService = mozilla::services::GetObserverService(); NS_ASSERTION(observerService, "needs observer service"); - nsresult rv = observerService->NotifyObservers(mObserverSubject, - CSP_VIOLATION_TOPIC, - mViolatedDirective.get()); + rv = observerService->NotifyObservers(mObserverSubject, + CSP_VIOLATION_TOPIC, + mViolatedDirective.get()); NS_ENSURE_SUCCESS(rv, rv); // 2) send reports for the policy that was violated diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index b73bea30a5..f17faddb70 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/dom/security/nsCSPUtils.cpp @@ -1264,6 +1264,12 @@ bool nsCSPDirective::equals(CSPDirective aDirective) const return (mDirective == aDirective); } +void +nsCSPDirective::getDirName(nsAString& outStr) const +{ + outStr.AppendASCII(CSP_CSPDirectiveToString(mDirective)); +} + /* =============== nsCSPChildSrcDirective ============= */ nsCSPChildSrcDirective::nsCSPChildSrcDirective(CSPDirective aDirective) @@ -1349,6 +1355,13 @@ nsBlockAllMixedContentDirective::toString(nsAString& outStr) const nsIContentSecurityPolicy::BLOCK_ALL_MIXED_CONTENT)); } +void +nsBlockAllMixedContentDirective::getDirName(nsAString& outStr) const +{ + outStr.AppendASCII(CSP_CSPDirectiveToString( + nsIContentSecurityPolicy::BLOCK_ALL_MIXED_CONTENT)); +} + /* =============== nsUpgradeInsecureDirective ============= */ nsUpgradeInsecureDirective::nsUpgradeInsecureDirective(CSPDirective aDirective) @@ -1367,6 +1380,13 @@ nsUpgradeInsecureDirective::toString(nsAString& outStr) const nsIContentSecurityPolicy::UPGRADE_IF_INSECURE_DIRECTIVE)); } +void +nsUpgradeInsecureDirective::getDirName(nsAString& outStr) const +{ + outStr.AppendASCII(CSP_CSPDirectiveToString( + nsIContentSecurityPolicy::UPGRADE_IF_INSECURE_DIRECTIVE)); +} + /* ===== nsRequireSRIForDirective ========================= */ nsRequireSRIForDirective::nsRequireSRIForDirective(CSPDirective aDirective) @@ -1418,6 +1438,13 @@ nsRequireSRIForDirective::allows(enum CSPKeyword aKeyword, const nsAString& aHas return (aKeyword != CSP_REQUIRE_SRI_FOR); } +void +nsRequireSRIForDirective::getDirName(nsAString& outStr) const +{ + outStr.AppendASCII(CSP_CSPDirectiveToString( + nsIContentSecurityPolicy::REQUIRE_SRI_FOR)); +} + /* ===== nsCSPPolicy ========================= */ nsCSPPolicy::nsCSPPolicy() @@ -1471,7 +1498,7 @@ nsCSPPolicy::permits(CSPDirective aDir, if (mDirectives[i]->equals(aDir)) { if (!mDirectives[i]->permits(aUri, aNonce, aWasRedirected, mReportOnly, mUpgradeInsecDir, aParserCreated)) { - mDirectives[i]->toString(outViolatedDirective); + mDirectives[i]->getDirName(outViolatedDirective); return false; } return true; @@ -1486,7 +1513,7 @@ nsCSPPolicy::permits(CSPDirective aDir, if (!aSpecific && defaultDir) { if (!defaultDir->permits(aUri, aNonce, aWasRedirected, mReportOnly, mUpgradeInsecDir, aParserCreated)) { - defaultDir->toString(outViolatedDirective); + defaultDir->getDirName(outViolatedDirective); return false; } return true; @@ -1612,7 +1639,7 @@ nsCSPPolicy::getDirectiveStringForContentType(nsContentPolicyType aContentType, nsCSPDirective* defaultDir = nullptr; for (uint32_t i = 0; i < mDirectives.Length(); i++) { if (mDirectives[i]->restrictsContentType(aContentType)) { - mDirectives[i]->toString(outDirective); + mDirectives[i]->getDirName(outDirective); return; } if (mDirectives[i]->isDefaultDirective()) { @@ -1622,7 +1649,7 @@ nsCSPPolicy::getDirectiveStringForContentType(nsContentPolicyType aContentType, // if we haven't found a matching directive yet, // the contentType must be restricted by the default directive if (defaultDir) { - defaultDir->toString(outDirective); + defaultDir->getDirName(outDirective); return; } NS_ASSERTION(false, "Can not query directive string for contentType!"); diff --git a/dom/security/nsCSPUtils.h b/dom/security/nsCSPUtils.h index b4c48fb42d..9b9ff46c90 100644 --- a/dom/security/nsCSPUtils.h +++ b/dom/security/nsCSPUtils.h @@ -494,6 +494,8 @@ class nsCSPDirective { bool visitSrcs(nsCSPSrcVisitor* aVisitor) const; + virtual void getDirName(nsAString& outStr) const; + protected: CSPDirective mDirective; nsTArray mSrcs; @@ -572,6 +574,8 @@ class nsBlockAllMixedContentDirective : public nsCSPDirective { void addSrcs(const nsTArray& aSrcs) { MOZ_ASSERT(false, "block-all-mixed-content does not hold any srcs"); } + + void getDirName(nsAString& outStr) const override; }; /* =============== nsUpgradeInsecureDirective === */ @@ -625,6 +629,8 @@ class nsUpgradeInsecureDirective : public nsCSPDirective { void addSrcs(const nsTArray& aSrcs) { MOZ_ASSERT(false, "upgrade-insecure-requests does not hold any srcs"); } + + void getDirName(nsAString& outStr) const override; }; /* ===== nsRequireSRIForDirective ========================= */ @@ -642,6 +648,7 @@ class nsRequireSRIForDirective : public nsCSPDirective { bool restrictsContentType(nsContentPolicyType aType) const; bool allows(enum CSPKeyword aKeyword, const nsAString& aHashOrNonce, bool aParserCreated) const; + void getDirName(nsAString& outStr) const override; private: nsTArray mTypes; From bfa8ea936819477106d16c3684484fea624ab7e4 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 4 Jan 2024 23:48:43 -0600 Subject: [PATCH 05/25] Issue #2402 - Sanitize the nsIPrincipal used to register a service worker to ensure CSP is not persisted. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ServiceWorkerManager.cpp | 33 ++++++++++++++++++++++-- dom/workers/ServiceWorkerRegisterJob.cpp | 5 ++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/dom/workers/ServiceWorkerManager.cpp b/dom/workers/ServiceWorkerManager.cpp index 6686e212ce..0bea0223f8 100644 --- a/dom/workers/ServiceWorkerManager.cpp +++ b/dom/workers/ServiceWorkerManager.cpp @@ -3154,10 +3154,12 @@ already_AddRefed ServiceWorkerManager::CreateNewRegistration(const nsCString& aScope, nsIPrincipal* aPrincipal) { + nsresult rv; + #ifdef DEBUG AssertIsOnMainThread(); nsCOMPtr scopeURI; - nsresult rv = NS_NewURI(getter_AddRefs(scopeURI), aScope, nullptr, nullptr); + rv = NS_NewURI(getter_AddRefs(scopeURI), aScope, nullptr, nullptr); MOZ_ASSERT(NS_SUCCEEDED(rv)); RefPtr tmp = @@ -3165,8 +3167,35 @@ ServiceWorkerManager::CreateNewRegistration(const nsCString& aScope, MOZ_ASSERT(!tmp); #endif + // The environment that registers the document may have some CSP applied + // to its principal. This should not be inherited by the registration + // itself or the worker it creates. To avoid confusion in callsites + // downstream we strip the CSP from the principal now. + // + // Unfortunately there is no API to clone a principal without its CSP. To + // achieve the same thing we serialize to the IPC PrincipalInfo type and + // back to an nsIPrincipal. + PrincipalInfo principalInfo; + rv = PrincipalToPrincipalInfo(aPrincipal, &principalInfo); + if (NS_WARN_IF(NS_FAILED(rv))) { + return nullptr; + } + + nsCOMPtr cleanPrincipal = + PrincipalInfoToPrincipal(principalInfo, &rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return nullptr; + } + + // Verify that we do not have any CSP set on our principal "clone". +#if defined(DEBUG) || !defined(RELEASE_OR_BETA) + nsCOMPtr csp; + MOZ_ALWAYS_SUCCEEDS(cleanPrincipal->GetCsp(getter_AddRefs(csp))); + MOZ_DIAGNOSTIC_ASSERT(!csp); +#endif + RefPtr registration = - new ServiceWorkerRegistrationInfo(aScope, aPrincipal); + new ServiceWorkerRegistrationInfo(aScope, cleanPrincipal); // From now on ownership of registration is with // mServiceWorkerRegistrationInfos. AddScopeAndRegistration(aScope, registration); diff --git a/dom/workers/ServiceWorkerRegisterJob.cpp b/dom/workers/ServiceWorkerRegisterJob.cpp index 30f0772ea6..6a97259d5f 100644 --- a/dom/workers/ServiceWorkerRegisterJob.cpp +++ b/dom/workers/ServiceWorkerRegisterJob.cpp @@ -52,6 +52,11 @@ ServiceWorkerRegisterJob::AsyncExecute() } } else { registration = swm->CreateNewRegistration(mScope, mPrincipal); + + if (!registration) { + FailUpdateJob(NS_ERROR_DOM_ABORT_ERR); + return; + } } SetRegistration(registration); From 1703d936afc8324c47db362d7c16ed2ca70f7e42 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Thu, 4 Jan 2024 23:57:26 -0600 Subject: [PATCH 06/25] Issue #2402 - ServiceWorker should not inherit CSP from registration principal. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ServiceWorkerPrivate.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/dom/workers/ServiceWorkerPrivate.cpp b/dom/workers/ServiceWorkerPrivate.cpp index fe6ec138b7..c66d98b0ea 100644 --- a/dom/workers/ServiceWorkerPrivate.cpp +++ b/dom/workers/ServiceWorkerPrivate.cpp @@ -1760,23 +1760,22 @@ ServiceWorkerPrivate::SpawnWorkerIfNeeded(WakeUpReason aWhy, info.mStorageAllowed = access > nsContentUtils::StorageAccess::ePrivateBrowsing; info.mOriginAttributes = mInfo->GetOriginAttributes(); + // The ServiceWorkerRegistration principal should never have any CSP + // set. The CSP from the page that registered the SW should not be + // inherited. Verify this is the case in non-release builds +#if defined(DEBUG) nsCOMPtr csp; rv = info.mPrincipal->GetCsp(getter_AddRefs(csp)); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + MOZ_DIAGNOSTIC_ASSERT(!csp); +#endif - info.mCSP = csp; - if (info.mCSP) { - rv = info.mCSP->GetAllowsEval(&info.mReportCSPViolations, - &info.mEvalAllowed); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } - } else { - info.mEvalAllowed = true; - info.mReportCSPViolations = false; - } + // Default CSP permissions for now. These will be overrided if necessary + // based on the script CSP headers during load in ScriptLoader. + info.mEvalAllowed = true; + info.mReportCSPViolations = false; WorkerPrivate::OverrideLoadInfoLoadGroup(info); From 971fc4b838f15e6c0273d75175d5faba80297e18 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 00:08:01 -0600 Subject: [PATCH 07/25] Issue #2402 - Improve ServiceWorker asserts and verify load principal does not inherit CSP. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ScriptLoader.cpp | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 123a46526a..3295160af3 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1279,6 +1279,7 @@ private: nsCOMPtr responsePrincipal = PrincipalInfoToPrincipal(*aPrincipalInfo); + MOZ_DIAGNOSTIC_ASSERT(responsePrincipal); nsIPrincipal* principal = mWorkerPrivate->GetPrincipal(); if (!principal) { @@ -1306,14 +1307,21 @@ private: mWorkerPrivate->SetBaseURI(finalURI); } - mozilla::DebugOnly principal = mWorkerPrivate->GetPrincipal(); - MOZ_ASSERT(principal); nsILoadGroup* loadGroup = mWorkerPrivate->GetLoadGroup(); - MOZ_ASSERT(loadGroup); + MOZ_DIAGNOSTIC_ASSERT(loadGroup); - mozilla::DebugOnly equal = false; - MOZ_ASSERT(responsePrincipal && NS_SUCCEEDED(responsePrincipal->Equals(principal, &equal))); - MOZ_ASSERT(equal); +#if defined(DEBUG) + nsIPrincipal* principal = mWorkerPrivate->GetPrincipal(); + MOZ_DIAGNOSTIC_ASSERT(principal); + + bool equal = false; + MOZ_ALWAYS_SUCCEEDS(responsePrincipal->Equals(principal, &equal)); + MOZ_DIAGNOSTIC_ASSERT(equal); + + nsCOMPtr csp; + MOZ_ALWAYS_SUCCEEDS(responsePrincipal->GetCsp(getter_AddRefs(csp))); + MOZ_DIAGNOSTIC_ASSERT(!csp); +#endif mWorkerPrivate->InitChannelInfo(aChannelInfo); mWorkerPrivate->SetPrincipal(responsePrincipal, loadGroup); From 37d4df19a8a1eccb9d0620bdf7297b1654d789cc Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 00:30:01 -0600 Subject: [PATCH 08/25] Issue #2402 - Factor out code to set WorkerPrivate CSP from headers. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ScriptLoader.cpp | 45 +++---------------------------- dom/workers/WorkerPrivate.cpp | 51 +++++++++++++++++++++++++++++++++++ dom/workers/WorkerPrivate.h | 4 +++ 3 files changed, 58 insertions(+), 42 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 3295160af3..6b036b3cff 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1204,48 +1204,9 @@ private: // We did inherit CSP in bug 1223647. If we do not already have a CSP, we // should get it from the HTTP headers on the worker script. if (!mWorkerPrivate->GetCSP() && CSPService::sCSPEnabled) { - NS_ConvertASCIItoUTF16 cspHeaderValue(tCspHeaderValue); - NS_ConvertASCIItoUTF16 cspROHeaderValue(tCspROHeaderValue); - - nsIPrincipal* principal = mWorkerPrivate->GetPrincipal(); - MOZ_ASSERT(principal, "Should not be null"); - - nsCOMPtr csp; - rv = principal->EnsureCSP(nullptr, getter_AddRefs(csp)); - - if (csp) { - // If there's a CSP header, apply it. - if (!cspHeaderValue.IsEmpty()) { - rv = CSP_AppendCSPFromHeader(csp, cspHeaderValue, false); - NS_ENSURE_SUCCESS(rv, rv); - } - // If there's a report-only CSP header, apply it. - if (!cspROHeaderValue.IsEmpty()) { - rv = CSP_AppendCSPFromHeader(csp, cspROHeaderValue, true); - NS_ENSURE_SUCCESS(rv, rv); - } - - // Set evalAllowed, default value is set in GetAllowsEval - bool evalAllowed = false; - bool reportEvalViolations = false; - rv = csp->GetAllowsEval(&reportEvalViolations, &evalAllowed); - NS_ENSURE_SUCCESS(rv, rv); - - mWorkerPrivate->SetCSP(csp); - mWorkerPrivate->SetEvalAllowed(evalAllowed); - mWorkerPrivate->SetReportCSPViolations(reportEvalViolations); - - // Set ReferrerPolicy, default value is set in GetReferrerPolicy - bool hasReferrerPolicy = false; - uint32_t rp = mozilla::net::RP_Default; - rv = csp->GetReferrerPolicy(&rp, &hasReferrerPolicy); - NS_ENSURE_SUCCESS(rv, rv); - - - if (hasReferrerPolicy) { //FIXME bug 1307366: move RP out of CSP code - mWorkerPrivate->SetReferrerPolicy(static_cast(rp)); - } - } + rv = mWorkerPrivate->SetCSPFromHeaderValues(tCspHeaderValue, + tCspROHeaderValue); + NS_ENSURE_SUCCESS(rv, rv); } if (parent) { // XHR Params Allowed diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 5e211c1085..01a8368c81 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -58,6 +58,7 @@ #include "mozilla/dom/MessageEventBinding.h" #include "mozilla/dom/MessagePort.h" #include "mozilla/dom/MessagePortBinding.h" +#include "mozilla/dom/nsCSPUtils.h" #include "mozilla/dom/Performance.h" #include "mozilla/dom/PMessagePort.h" #include "mozilla/dom/Promise.h" @@ -2382,6 +2383,56 @@ WorkerPrivateParent::GetDocument() const return nullptr; } +template +nsresult +WorkerPrivateParent::SetCSPFromHeaderValues(const nsACString& aCSPHeaderValue, + const nsACString& aCSPReportOnlyHeaderValue) +{ + AssertIsOnMainThread(); + MOZ_DIAGNOSTIC_ASSERT(!mLoadInfo.mCSP); + + NS_ConvertASCIItoUTF16 cspHeaderValue(aCSPHeaderValue); + NS_ConvertASCIItoUTF16 cspROHeaderValue(aCSPReportOnlyHeaderValue); + + nsCOMPtr csp; + nsresult rv = mLoadInfo.mPrincipal->EnsureCSP(nullptr, getter_AddRefs(csp)); + if (!csp) { + return NS_OK; + } + + // If there's a CSP header, apply it. + if (!cspHeaderValue.IsEmpty()) { + rv = CSP_AppendCSPFromHeader(csp, cspHeaderValue, false); + NS_ENSURE_SUCCESS(rv, rv); + } + // If there's a report-only CSP header, apply it. + if (!cspROHeaderValue.IsEmpty()) { + rv = CSP_AppendCSPFromHeader(csp, cspROHeaderValue, true); + NS_ENSURE_SUCCESS(rv, rv); + } + + // Set evalAllowed, default value is set in GetAllowsEval + bool evalAllowed = false; + bool reportEvalViolations = false; + rv = csp->GetAllowsEval(&reportEvalViolations, &evalAllowed); + NS_ENSURE_SUCCESS(rv, rv); + + // Set ReferrerPolicy, default value is set in GetReferrerPolicy + bool hasReferrerPolicy = false; + uint32_t rp = mozilla::net::RP_Unset; + rv = csp->GetReferrerPolicy(&rp, &hasReferrerPolicy); + NS_ENSURE_SUCCESS(rv, rv); + + mLoadInfo.mCSP = csp; + mLoadInfo.mEvalAllowed = evalAllowed; + mLoadInfo.mReportCSPViolations = reportEvalViolations; + + if (hasReferrerPolicy) { + mLoadInfo.mReferrerPolicy = static_cast(rp); + } + + return NS_OK; +} // Can't use NS_IMPL_CYCLE_COLLECTION_CLASS(WorkerPrivateParent) because of the // templates. diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 26afecb69b..b82e994028 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -701,6 +701,10 @@ public: mLoadInfo.mCSP = aCSP; } + nsresult + SetCSPFromHeaderValues(const nsACString& aCSPHeaderValue, + const nsACString& aCSPReportOnlyHeaderValue); + net::ReferrerPolicy GetReferrerPolicy() const { From 15875f3d8c3ec23eadae03bd12ef889147aa5573 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 00:42:52 -0600 Subject: [PATCH 09/25] Issue #2402 - Apply CSP headers to ServiceWorker when loading offlined script. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ScriptLoader.cpp | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 6b036b3cff..ae023b520e 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -464,6 +464,8 @@ private: nsCOMPtr mBaseURI; mozilla::dom::ChannelInfo mChannelInfo; UniquePtr mPrincipalInfo; + nsCString mCSPHeaderValue; + nsCString mCSPReportOnlyHeaderValue; }; NS_IMPL_ISUPPORTS(CacheScriptLoader, nsIStreamLoaderObserver) @@ -1231,7 +1233,9 @@ private: DataReceivedFromCache(uint32_t aIndex, const uint8_t* aString, uint32_t aStringLen, const mozilla::dom::ChannelInfo& aChannelInfo, - UniquePtr aPrincipalInfo) + UniquePtr aPrincipalInfo, + const nsACString& aCSPHeaderValue, + const nsACString& aCSPReportOnlyHeaderValue) { AssertIsOnMainThread(); MOZ_ASSERT(aIndex < mLoadInfos.Length()); @@ -1286,6 +1290,10 @@ private: mWorkerPrivate->InitChannelInfo(aChannelInfo); mWorkerPrivate->SetPrincipal(responsePrincipal, loadGroup); + + rv = mWorkerPrivate->SetCSPFromHeaderValues(aCSPHeaderValue, + aCSPReportOnlyHeaderValue); + MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); } if (NS_SUCCEEDED(rv)) { @@ -1699,6 +1707,14 @@ CacheScriptLoader::ResolvedCallback(JSContext* aCx, return; } + InternalHeaders* headers = response->GetInternalHeaders(); + + IgnoredErrorResult ignored; + headers->Get(NS_LITERAL_CSTRING("content-security-policy"), + mCSPHeaderValue, ignored); + headers->Get(NS_LITERAL_CSTRING("content-security-policy-report-only"), + mCSPReportOnlyHeaderValue, ignored); + nsCOMPtr inputStream; response->GetBody(getter_AddRefs(inputStream)); mChannelInfo = response->GetChannelInfo(); @@ -1710,7 +1726,8 @@ CacheScriptLoader::ResolvedCallback(JSContext* aCx, if (!inputStream) { mLoadInfo.mCacheStatus = ScriptLoadInfo::Cached; mRunnable->DataReceivedFromCache(mIndex, (uint8_t*)"", 0, mChannelInfo, - Move(mPrincipalInfo)); + Move(mPrincipalInfo), mCSPHeaderValue, + mCSPReportOnlyHeaderValue); return; } @@ -1770,7 +1787,8 @@ CacheScriptLoader::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aCont MOZ_ASSERT(mPrincipalInfo); mRunnable->DataReceivedFromCache(mIndex, aString, aStringLen, mChannelInfo, - Move(mPrincipalInfo)); + Move(mPrincipalInfo), mCSPHeaderValue, + mCSPReportOnlyHeaderValue); return NS_OK; } From 14f0bc5e7c6f8f662e1c40ad6b9e00ee35b1d2f4 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 00:57:40 -0600 Subject: [PATCH 10/25] Issue #2402 - Move code to fill InternalHeaders from an nsIChannel response into utility method. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/fetch/FetchDriver.cpp | 39 +------------------------------ dom/fetch/InternalHeaders.cpp | 43 +++++++++++++++++++++++++++++++++++ dom/fetch/InternalHeaders.h | 1 + 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp index 2a0bb1ea2c..0198cc44aa 100644 --- a/dom/fetch/FetchDriver.cpp +++ b/dom/fetch/FetchDriver.cpp @@ -12,7 +12,6 @@ #include "nsIOutputStream.h" #include "nsIHttpChannel.h" #include "nsIHttpChannelInternal.h" -#include "nsIHttpHeaderVisitor.h" #include "nsIScriptSecurityManager.h" #include "nsIThreadRetargetableRequest.h" #include "nsIUploadChannel2.h" @@ -449,38 +448,6 @@ FetchDriver::FailWithNetworkError() mChannel = nullptr; } -namespace { -class FillResponseHeaders final : public nsIHttpHeaderVisitor { - InternalResponse* mResponse; - - ~FillResponseHeaders() - { } -public: - NS_DECL_ISUPPORTS - - explicit FillResponseHeaders(InternalResponse* aResponse) - : mResponse(aResponse) - { - } - - NS_IMETHOD - VisitHeader(const nsACString & aHeader, const nsACString & aValue) override - { - ErrorResult result; - mResponse->Headers()->Append(aHeader, aValue, result); - if (result.Failed()) { - NS_WARNING(nsPrintfCString("Fetch ignoring illegal header - '%s': '%s'", - PromiseFlatCString(aHeader).get(), - PromiseFlatCString(aValue).get()).get()); - result.SuppressException(); - } - return NS_OK; - } -}; - -NS_IMPL_ISUPPORTS(FillResponseHeaders, nsIHttpHeaderVisitor) -} // namespace - NS_IMETHODIMP FetchDriver::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) @@ -540,11 +507,7 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest, response = new InternalResponse(responseStatus, statusText, mRequest->GetCredentialsMode()); - RefPtr visitor = new FillResponseHeaders(response); - rv = httpChannel->VisitResponseHeaders(visitor); - if (NS_WARN_IF(NS_FAILED(rv))) { - NS_WARNING("Failed to visit all headers."); - } + response->Headers()->FillResponseHeaders(httpChannel); // If Content-Encoding or Transfer-Encoding headers are set, then the actual // Content-Length (which refer to the decoded data) is obscured behind the encodings. diff --git a/dom/fetch/InternalHeaders.cpp b/dom/fetch/InternalHeaders.cpp index f4851b7064..54d4da093d 100644 --- a/dom/fetch/InternalHeaders.cpp +++ b/dom/fetch/InternalHeaders.cpp @@ -11,6 +11,7 @@ #include "nsCharSeparatedTokenizer.h" #include "nsContentUtils.h" +#include "nsIHttpHeaderVisitor.h" #include "nsNetUtil.h" #include "nsReadableUtils.h" @@ -478,6 +479,48 @@ InternalHeaders::Fill(const Record& aInit, ErrorResult& aR } } +namespace { + +class FillHeaders final : public nsIHttpHeaderVisitor +{ + RefPtr mInternalHeaders; + + ~FillHeaders() = default; + +public: + NS_DECL_ISUPPORTS + + explicit FillHeaders(InternalHeaders* aInternalHeaders) + : mInternalHeaders(aInternalHeaders) + { + MOZ_DIAGNOSTIC_ASSERT(mInternalHeaders); + } + + NS_IMETHOD + VisitHeader(const nsACString& aHeader, const nsACString& aValue) override + { + IgnoredErrorResult result; + mInternalHeaders->Append(aHeader, aValue, result); + return NS_OK; + } +}; + +NS_IMPL_ISUPPORTS(FillHeaders, nsIHttpHeaderVisitor) + +} // namespace + +void +InternalHeaders::FillResponseHeaders(nsIRequest* aRequest) +{ + nsCOMPtr httpChannel = do_QueryInterface(aRequest); + if (!httpChannel) { + return; + } + + RefPtr visitor = new FillHeaders(this); + httpChannel->VisitResponseHeaders(visitor); +} + bool InternalHeaders::HasOnlySimpleHeaders() const { diff --git a/dom/fetch/InternalHeaders.h b/dom/fetch/InternalHeaders.h index 0a6996ab38..c9aff2d4d9 100644 --- a/dom/fetch/InternalHeaders.h +++ b/dom/fetch/InternalHeaders.h @@ -114,6 +114,7 @@ public: void Fill(const InternalHeaders& aInit, ErrorResult& aRv); void Fill(const Sequence>& aInit, ErrorResult& aRv); void Fill(const Record& aInit, ErrorResult& aRv); + void FillResponseHeaders(nsIRequest* aRequest); bool HasOnlySimpleHeaders() const; From b723bb60adc6c15af569d0a8a7440e5793c5fabe Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 01:11:53 -0600 Subject: [PATCH 11/25] Issue #2402 - Persist response headers for offlined service worker scripts. https://bugzilla.mozilla.org/show_bug.cgi?id=1337543 --- dom/workers/ScriptLoader.cpp | 1 + dom/workers/ServiceWorkerScriptCache.cpp | 22 ++++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index ae023b520e..bdc4efed23 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -693,6 +693,7 @@ private: } ir->SetPrincipalInfo(Move(principalInfo)); + ir->Headers()->FillResponseHeaders(loadInfo.mChannel); RefPtr response = new mozilla::dom::Response(mCacheCreator->Global(), ir, nullptr); diff --git a/dom/workers/ServiceWorkerScriptCache.cpp b/dom/workers/ServiceWorkerScriptCache.cpp index f343c35586..00e0446edf 100644 --- a/dom/workers/ServiceWorkerScriptCache.cpp +++ b/dom/workers/ServiceWorkerScriptCache.cpp @@ -239,6 +239,7 @@ public: CompareCallback* aCallback) : mRegistration(aRegistration) , mCallback(aCallback) + , mInternalHeaders(new InternalHeaders()) , mState(WaitingForOpen) , mNetworkFinished(false) , mCacheFinished(false) @@ -425,11 +426,21 @@ public: return mCacheStorage; } - void - InitChannelInfo(nsIChannel* aChannel) + nsresult + OnStartRequest(nsIChannel* aChannel) { + nsresult rv = SetPrincipalInfo(aChannel); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + mChannelInfo.InitFromChannel(aChannel); + + mInternalHeaders->FillResponseHeaders(aChannel); + + return NS_OK; } + nsresult SetPrincipalInfo(nsIChannel* aChannel) @@ -553,6 +564,9 @@ private: ir->SetPrincipalInfo(Move(mPrincipalInfo)); } + IgnoredErrorResult ignored; + ir->Headers()->Fill(*mInternalHeaders, ignored); + RefPtr response = new Response(aCache->GetGlobalObject(), ir, nullptr); RequestOrUSVString request; @@ -587,6 +601,7 @@ private: nsString mNewCacheName; ChannelInfo mChannelInfo; + RefPtr mInternalHeaders; UniquePtr mPrincipalInfo; @@ -682,8 +697,7 @@ CompareNetwork::OnStartRequest(nsIRequest* aRequest, nsISupports* aContext) MOZ_ASSERT(channel == mChannel); #endif - mManager->InitChannelInfo(mChannel); - nsresult rv = mManager->SetPrincipalInfo(mChannel); + nsresult rv = mManager->OnStartRequest(mChannel); if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } From f8a174e19fc0ea1ded4499dfd6252dff1b1cb931 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Fri, 5 Jan 2024 02:18:41 -0600 Subject: [PATCH 12/25] Issue #2402 - Move WorkerPrivate::SetPrincipal() logic into a WorkerLoadInfo method. https://bugzilla.mozilla.org/show_bug.cgi?id=1333573 Make worker SetPrincipalOnMainThread fallible. https://bugzilla.mozilla.org/show_bug.cgi?id=1338523 --- dom/workers/ScriptLoader.cpp | 11 ++++-- dom/workers/ServiceWorkerPrivate.cpp | 5 +++ dom/workers/WorkerPrivate.cpp | 52 ++++++++++++++++------------ dom/workers/WorkerPrivate.h | 4 +-- dom/workers/Workers.h | 2 ++ 5 files changed, 47 insertions(+), 27 deletions(-) diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index bdc4efed23..758c1660e7 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1202,7 +1202,7 @@ private: // load group's appId and browser element flag. MOZ_ASSERT(NS_LoadGroupMatchesPrincipal(channelLoadGroup, channelPrincipal)); - mWorkerPrivate->SetPrincipal(channelPrincipal, channelLoadGroup); + mWorkerPrivate->SetPrincipalOnMainThread(channelPrincipal, channelLoadGroup); // We did inherit CSP in bug 1223647. If we do not already have a CSP, we // should get it from the HTTP headers on the worker script. @@ -1290,7 +1290,14 @@ private: #endif mWorkerPrivate->InitChannelInfo(aChannelInfo); - mWorkerPrivate->SetPrincipal(responsePrincipal, loadGroup); + + // Override the principal on the WorkerPrivate. We just asserted that + // this is the same as our current WorkerPrivate principal, so this is + // almost a no-op. We must do, it though, in order to avoid accidentally + // propagating the CSP object back to the ServiceWorkerRegistration + // principal. If bug 965637 is fixed then this can be removed. + rv = mWorkerPrivate->SetPrincipalOnMainThread(responsePrincipal, loadGroup); + MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv)); rv = mWorkerPrivate->SetCSPFromHeaderValues(aCSPHeaderValue, aCSPReportOnlyHeaderValue); diff --git a/dom/workers/ServiceWorkerPrivate.cpp b/dom/workers/ServiceWorkerPrivate.cpp index c66d98b0ea..f30ae67a19 100644 --- a/dom/workers/ServiceWorkerPrivate.cpp +++ b/dom/workers/ServiceWorkerPrivate.cpp @@ -1779,6 +1779,11 @@ ServiceWorkerPrivate::SpawnWorkerIfNeeded(WakeUpReason aWhy, WorkerPrivate::OverrideLoadInfoLoadGroup(info); + rv = info.SetPrincipalOnMainThread(info.mPrincipal, info.mLoadGroup); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + AutoJSAPI jsapi; jsapi.Init(); ErrorResult error; diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 01a8368c81..7e96fc6ad3 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -3679,47 +3679,53 @@ WorkerPrivateParent::SetBaseURI(nsIURI* aBaseURI) nsContentUtils::GetUTFOrigin(aBaseURI, mLocationInfo.mOrigin); } -template -void -WorkerPrivateParent::SetPrincipal(nsIPrincipal* aPrincipal, - nsILoadGroup* aLoadGroup) +nsresult +WorkerLoadInfo::SetPrincipalOnMainThread(nsIPrincipal* aPrincipal, + nsILoadGroup* aLoadGroup) { AssertIsOnMainThread(); MOZ_ASSERT(NS_LoadGroupMatchesPrincipal(aLoadGroup, aPrincipal)); - MOZ_ASSERT(!mLoadInfo.mPrincipalInfo); - mLoadInfo.mPrincipal = aPrincipal; - mLoadInfo.mPrincipalIsSystem = nsContentUtils::IsSystemPrincipal(aPrincipal); + mPrincipal = aPrincipal; + mPrincipalIsSystem = nsContentUtils::IsSystemPrincipal(aPrincipal); - aPrincipal->GetCsp(getter_AddRefs(mLoadInfo.mCSP)); + nsresult rv = aPrincipal->GetCsp(getter_AddRefs(mCSP)); + NS_ENSURE_SUCCESS(rv, rv); - if (mLoadInfo.mCSP) { - mLoadInfo.mCSP->GetAllowsEval(&mLoadInfo.mReportCSPViolations, - &mLoadInfo.mEvalAllowed); + if (mCSP) { + mCSP->GetAllowsEval(&mReportCSPViolations, &mEvalAllowed); // Set ReferrerPolicy bool hasReferrerPolicy = false; - uint32_t rp = mozilla::net::RP_Default; + uint32_t rp = mozilla::net::RP_Unset; - nsresult rv = mLoadInfo.mCSP->GetReferrerPolicy(&rp, &hasReferrerPolicy); - NS_ENSURE_SUCCESS_VOID(rv); + rv = mCSP->GetReferrerPolicy(&rp, &hasReferrerPolicy); + NS_ENSURE_SUCCESS(rv, rv); if (hasReferrerPolicy) { - mLoadInfo.mReferrerPolicy = static_cast(rp); + mReferrerPolicy = static_cast(rp); } } else { - mLoadInfo.mEvalAllowed = true; - mLoadInfo.mReportCSPViolations = false; + mEvalAllowed = true; + mReportCSPViolations = false; } - mLoadInfo.mLoadGroup = aLoadGroup; + mLoadGroup = aLoadGroup; - mLoadInfo.mPrincipalInfo = new PrincipalInfo(); - mLoadInfo.mOriginAttributes = nsContentUtils::GetOriginAttributes(aLoadGroup); + mPrincipalInfo = new PrincipalInfo(); + mOriginAttributes = nsContentUtils::GetOriginAttributes(aLoadGroup); - nsContentUtils::GetUTFOrigin(aPrincipal, mLoadInfo.mOrigin); + rv = PrincipalToPrincipalInfo(aPrincipal, mPrincipalInfo); + NS_ENSURE_SUCCESS(rv, rv); - MOZ_ALWAYS_SUCCEEDS( - PrincipalToPrincipalInfo(aPrincipal, mLoadInfo.mPrincipalInfo)); + return NS_OK; +} + +template +nsresult +WorkerPrivateParent::SetPrincipalOnMainThread(nsIPrincipal* aPrincipal, + nsILoadGroup* aLoadGroup) +{ + return mLoadInfo.SetPrincipalOnMainThread(aPrincipal, aLoadGroup); } template diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index b82e994028..1c9cb10ded 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -656,8 +656,8 @@ public: return mLoadInfo.mPrincipal; } - void - SetPrincipal(nsIPrincipal* aPrincipal, nsILoadGroup* aLoadGroup); + nsresult + SetPrincipalOnMainThread(nsIPrincipal* aPrincipal, nsILoadGroup* aLoadGroup); bool UsesSystemPrincipal() const diff --git a/dom/workers/Workers.h b/dom/workers/Workers.h index 6b0a0158d2..541dfed6aa 100644 --- a/dom/workers/Workers.h +++ b/dom/workers/Workers.h @@ -268,6 +268,8 @@ struct WorkerLoadInfo ~WorkerLoadInfo(); void StealFrom(WorkerLoadInfo& aOther); + + nsresult SetPrincipalOnMainThread(nsIPrincipal* aPrincipal, nsILoadGroup* aLoadGroup); }; // All of these are implemented in RuntimeService.cpp From 6979441734d52562895fe030d7604047f498f875 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sat, 6 Jan 2024 15:29:04 -0600 Subject: [PATCH 13/25] Issue #2402 - importScripts should be governed by script-src in Web Workers. https://bugzilla.mozilla.org/show_bug.cgi?id=1322111 Add TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS content policy. Update the Cache API schema to account for new nsIContentPolicy type. --- dom/base/nsContentPolicyUtils.h | 1 + dom/base/nsContentUtils.cpp | 1 + dom/base/nsIContentPolicyBase.idl | 8 ++++++++ dom/cache/DBSchema.cpp | 18 ++++++++++++++++-- dom/fetch/InternalRequest.cpp | 1 + dom/security/nsCSPUtils.cpp | 1 + dom/workers/ScriptLoader.cpp | 17 +++++++++++------ extensions/permissions/nsContentBlocker.cpp | 1 + 8 files changed, 40 insertions(+), 8 deletions(-) diff --git a/dom/base/nsContentPolicyUtils.h b/dom/base/nsContentPolicyUtils.h index 600b24c56b..3984ede544 100644 --- a/dom/base/nsContentPolicyUtils.h +++ b/dom/base/nsContentPolicyUtils.h @@ -135,6 +135,7 @@ NS_CP_ContentTypeName(uint32_t contentType) CASE_RETURN( TYPE_INTERNAL_STYLESHEET ); CASE_RETURN( TYPE_INTERNAL_STYLESHEET_PRELOAD ); CASE_RETURN( TYPE_SAVEAS_DOWNLOAD ); + CASE_RETURN( TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS ); default: return ""; } diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index b246132de2..db8ea5fed7 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -8595,6 +8595,7 @@ nsContentUtils::InternalContentPolicyTypeToExternal(nsContentPolicyType aType) case nsIContentPolicy::TYPE_INTERNAL_WORKER: case nsIContentPolicy::TYPE_INTERNAL_SHARED_WORKER: case nsIContentPolicy::TYPE_INTERNAL_SERVICE_WORKER: + case nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS: return nsIContentPolicy::TYPE_SCRIPT; case nsIContentPolicy::TYPE_INTERNAL_EMBED: diff --git a/dom/base/nsIContentPolicyBase.idl b/dom/base/nsIContentPolicyBase.idl index 184257d11d..589229cd3d 100644 --- a/dom/base/nsIContentPolicyBase.idl +++ b/dom/base/nsIContentPolicyBase.idl @@ -333,6 +333,14 @@ interface nsIContentPolicyBase : nsISupports */ const nsContentPolicyType TYPE_SAVEAS_DOWNLOAD = 42; + /** + * Indicates an importScripts() inside a worker script. + * + * This will be mapped to TYPE_SCRIPT before being passed to content policy + * implementations. + */ + const nsContentPolicyType TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS = 43; + /* When adding new content types, please update nsContentBlocker, * NS_CP_ContentTypeName, nsCSPContext, CSP_ContentTypeToDirective, * DoContentSecurityChecks, all nsIContentPolicy implementations, the diff --git a/dom/cache/DBSchema.cpp b/dom/cache/DBSchema.cpp index 2025150380..953aacb14e 100644 --- a/dom/cache/DBSchema.cpp +++ b/dom/cache/DBSchema.cpp @@ -34,7 +34,7 @@ namespace db { const int32_t kFirstShippedSchemaVersion = 15; namespace { // Update this whenever the DB schema is changed. -const int32_t kLatestSchemaVersion = 24; +const int32_t kLatestSchemaVersion = 25; // --------- // The following constants define the SQL schema. These are defined in the // same order the SQL should be executed in CreateOrMigrateSchema(). They are @@ -287,7 +287,8 @@ static_assert(nsIContentPolicy::TYPE_INVALID == 0 && nsIContentPolicy::TYPE_INTERNAL_STYLESHEET == 39 && nsIContentPolicy::TYPE_INTERNAL_STYLESHEET_PRELOAD == 40 && nsIContentPolicy::TYPE_INTERNAL_IMAGE_FAVICON == 41 && - nsIContentPolicy::TYPE_SAVEAS_DOWNLOAD == 42, + nsIContentPolicy::TYPE_SAVEAS_DOWNLOAD == 42 && + nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS == 43, "nsContentPolicyType values are as expected"); namespace { @@ -2478,6 +2479,7 @@ nsresult MigrateFrom20To21(mozIStorageConnection* aConn, bool& aRewriteSchema); nsresult MigrateFrom21To22(mozIStorageConnection* aConn, bool& aRewriteSchema); nsresult MigrateFrom22To23(mozIStorageConnection* aConn, bool& aRewriteSchema); nsresult MigrateFrom23To24(mozIStorageConnection* aConn, bool& aRewriteSchema); +nsresult MigrateFrom24To25(mozIStorageConnection* aConn, bool& aRewriteSchema); // Configure migration functions to run for the given starting version. Migration sMigrationList[] = { Migration(15, MigrateFrom15To16), @@ -2489,6 +2491,7 @@ Migration sMigrationList[] = { Migration(21, MigrateFrom21To22), Migration(22, MigrateFrom22To23), Migration(23, MigrateFrom23To24), + Migration(24, MigrateFrom24To25), }; uint32_t sMigrationListLength = sizeof(sMigrationList) / sizeof(Migration); nsresult @@ -3013,6 +3016,17 @@ nsresult MigrateFrom23To24(mozIStorageConnection* aConn, bool& aRewriteSchema) return rv; } +nsresult MigrateFrom24To25(mozIStorageConnection* aConn, bool& aRewriteSchema) +{ + MOZ_ASSERT(!NS_IsMainThread()); + MOZ_DIAGNOSTIC_ASSERT(aConn); + + // The only change between 24 and 25 was a new nsIContentPolicy type. + nsresult rv = aConn->SetSchemaVersion(25); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + return rv; +} + } // anonymous namespace } // namespace db } // namespace cache diff --git a/dom/fetch/InternalRequest.cpp b/dom/fetch/InternalRequest.cpp index 71a5590aae..36df242b46 100644 --- a/dom/fetch/InternalRequest.cpp +++ b/dom/fetch/InternalRequest.cpp @@ -234,6 +234,7 @@ InternalRequest::MapContentPolicyTypeToRequestContext(nsContentPolicyType aConte case nsIContentPolicy::TYPE_INTERNAL_SCRIPT: case nsIContentPolicy::TYPE_INTERNAL_SCRIPT_PRELOAD: case nsIContentPolicy::TYPE_INTERNAL_SERVICE_WORKER: + case nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS: context = RequestContext::Script; break; case nsIContentPolicy::TYPE_INTERNAL_WORKER: diff --git a/dom/security/nsCSPUtils.cpp b/dom/security/nsCSPUtils.cpp index f17faddb70..cb04db315d 100644 --- a/dom/security/nsCSPUtils.cpp +++ b/dom/security/nsCSPUtils.cpp @@ -212,6 +212,7 @@ CSP_ContentTypeToDirective(nsContentPolicyType aType) case nsIContentPolicy::TYPE_SCRIPT: case nsIContentPolicy::TYPE_INTERNAL_SCRIPT: case nsIContentPolicy::TYPE_INTERNAL_SCRIPT_PRELOAD: + case nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS: return nsIContentSecurityPolicy::SCRIPT_SRC_DIRECTIVE; case nsIContentPolicy::TYPE_STYLESHEET: diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 758c1660e7..0a9b7207e7 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -112,7 +112,7 @@ ChannelFromScriptURL(nsIPrincipal* principal, const nsAString& aScriptURL, bool aIsMainScript, WorkerScriptType aWorkerScriptType, - nsContentPolicyType aContentPolicyType, + nsContentPolicyType aMainScriptContentPolicyType, nsLoadFlags aLoadFlags, bool aDefaultURIEncoding, nsIChannel** aChannel) @@ -169,6 +169,10 @@ ChannelFromScriptURL(nsIPrincipal* principal, secFlags = nsILoadInfo::SEC_ALLOW_CROSS_ORIGIN_DATA_IS_NULL; } + nsContentPolicyType contentPolicyType = + aIsMainScript ? aMainScriptContentPolicyType + : nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS; + nsCOMPtr channel; // If we have the document, use it. Unfortunately, for dedicated workers // 'parentDoc' ends up being the parent document, which is not the document @@ -179,7 +183,7 @@ ChannelFromScriptURL(nsIPrincipal* principal, uri, parentDoc, secFlags, - aContentPolicyType, + contentPolicyType, loadGroup, nullptr, // aCallbacks aLoadFlags, @@ -194,7 +198,7 @@ ChannelFromScriptURL(nsIPrincipal* principal, uri, principal, secFlags, - aContentPolicyType, + contentPolicyType, loadGroup, nullptr, // aCallbacks aLoadFlags, @@ -2165,7 +2169,7 @@ ChannelFromScriptURLMainThread(nsIPrincipal* aPrincipal, nsIDocument* aParentDoc, nsILoadGroup* aLoadGroup, const nsAString& aScriptURL, - nsContentPolicyType aContentPolicyType, + nsContentPolicyType aMainScriptContentPolicyType, bool aDefaultURIEncoding, nsIChannel** aChannel) { @@ -2178,8 +2182,9 @@ ChannelFromScriptURLMainThread(nsIPrincipal* aPrincipal, return ChannelFromScriptURL(aPrincipal, aBaseURI, aParentDoc, aLoadGroup, ios, secMan, aScriptURL, true, WorkerScript, - aContentPolicyType, nsIRequest::LOAD_NORMAL, - aDefaultURIEncoding, aChannel); + aMainScriptContentPolicyType, + nsIRequest::LOAD_NORMAL, aDefaultURIEncoding, + aChannel); } nsresult diff --git a/extensions/permissions/nsContentBlocker.cpp b/extensions/permissions/nsContentBlocker.cpp index 391785dc3a..29416090a9 100644 --- a/extensions/permissions/nsContentBlocker.cpp +++ b/extensions/permissions/nsContentBlocker.cpp @@ -67,6 +67,7 @@ static const char *kTypeString[] = { "", // TYPE_INTERNAL_STYLESHEET_PRELOAD "", // TYPE_INTERNAL_IMAGE_FAVICON "saveas_download", + "", // TYPE_INTERNAL_WORKERS_IMPORT_SCRIPTS }; #define NUMBER_OF_TYPES MOZ_ARRAY_LENGTH(kTypeString) From 5202c08cfb4a6c2b6eae21cd981b94448ac65c42 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 7 Jan 2024 07:34:05 -0600 Subject: [PATCH 14/25] Issue #2402 - Don't call WorkerPrivate::WorkerName() to get service worker scope. https://bugzilla.mozilla.org/show_bug.cgi?id=1350433 --- dom/console/Console.cpp | 2 +- dom/fetch/Fetch.cpp | 2 +- dom/workers/ServiceWorkerEvents.cpp | 2 +- dom/workers/WorkerPrivate.cpp | 6 +++--- dom/workers/WorkerPrivate.h | 9 ++++++++- 5 files changed, 14 insertions(+), 7 deletions(-) diff --git a/dom/console/Console.cpp b/dom/console/Console.cpp index 715e9fe84d..b5e3565dba 100755 --- a/dom/console/Console.cpp +++ b/dom/console/Console.cpp @@ -598,7 +598,7 @@ private: innerID = NS_LITERAL_STRING("ServiceWorker"); // Use scope as ID so the webconsole can decide if the message should // show up per tab - id.AssignWithConversion(mWorkerPrivate->WorkerName()); + id.AssignWithConversion(mWorkerPrivate->ServiceWorkerScope()); } else { innerID = NS_LITERAL_STRING("Worker"); } diff --git a/dom/fetch/Fetch.cpp b/dom/fetch/Fetch.cpp index a9d8514765..0b31e0c84b 100644 --- a/dom/fetch/Fetch.cpp +++ b/dom/fetch/Fetch.cpp @@ -710,7 +710,7 @@ WorkerFetchResolver::FlushConsoleReport() return; } - swm->FlushReportsToAllClients(worker->WorkerName(), mReporter); + swm->FlushReportsToAllClients(worker->ServiceWorkerScope(), mReporter); return; } diff --git a/dom/workers/ServiceWorkerEvents.cpp b/dom/workers/ServiceWorkerEvents.cpp index 569422da25..f017f2936a 100644 --- a/dom/workers/ServiceWorkerEvents.cpp +++ b/dom/workers/ServiceWorkerEvents.cpp @@ -805,7 +805,7 @@ public: WaitUntilHandler(WorkerPrivate* aWorkerPrivate, JSContext* aCx) : mWorkerPrivate(aWorkerPrivate) - , mScope(mWorkerPrivate->WorkerName()) + , mScope(mWorkerPrivate->ServiceWorkerScope()) , mLine(0) , mColumn(0) { diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 7e96fc6ad3..7a0c272f56 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -550,7 +550,7 @@ private: RefPtr swm = ServiceWorkerManager::GetInstance(); if (swm) { swm->HandleError(aCx, aWorkerPrivate->GetPrincipal(), - aWorkerPrivate->WorkerName(), + aWorkerPrivate->ServiceWorkerScope(), aWorkerPrivate->ScriptURL(), EmptyString(), EmptyString(), EmptyString(), 0, 0, JSREPORT_ERROR, JSEXN_ERR); @@ -1255,7 +1255,7 @@ private: RefPtr swm = ServiceWorkerManager::GetInstance(); if (swm) { swm->HandleError(aCx, aWorkerPrivate->GetPrincipal(), - aWorkerPrivate->WorkerName(), + aWorkerPrivate->ServiceWorkerScope(), aWorkerPrivate->ScriptURL(), mReport.mMessage, mReport.mFilename, mReport.mLine, mReport.mLineNumber, @@ -6727,7 +6727,7 @@ WorkerPrivate::GetOrCreateGlobalScope(JSContext* aCx) if (IsSharedWorker()) { globalScope = new SharedWorkerGlobalScope(this, WorkerName()); } else if (IsServiceWorker()) { - globalScope = new ServiceWorkerGlobalScope(this, WorkerName()); + globalScope = new ServiceWorkerGlobalScope(this, ServiceWorkerScope()); } else { globalScope = new DedicatedWorkerGlobalScope(this); } diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index 1c9cb10ded..dda81beb57 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -535,6 +535,13 @@ public: return mLoadInfo.mServiceWorkerID; } + const nsCString& + ServiceWorkerScope() const + { + MOZ_DIAGNOSTIC_ASSERT(IsServiceWorker()); + return mWorkerName; + } + nsIURI* GetBaseURI() const { @@ -831,7 +838,7 @@ public: const nsCString& WorkerName() const { - MOZ_ASSERT(IsServiceWorker() || IsSharedWorker()); + MOZ_ASSERT(IsSharedWorker()); return mWorkerName; } From b7abd4a1270779b49bb4f4315efb93710e5469a9 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 7 Jan 2024 07:37:47 -0600 Subject: [PATCH 15/25] Issue #2402 - Require ServiceWorkerGlobalScope.importScripts() to only accept JavaScript MIME types. https://bugzilla.mozilla.org/show_bug.cgi?id=1354577 Require ServiceWorkerContainer.register() to only accept JavaScript MIME types. --- dom/locales/en-US/chrome/dom/dom.properties | 2 +- dom/workers/ScriptLoader.cpp | 29 +++++++++++++++++++++ dom/workers/ServiceWorkerScriptCache.cpp | 7 +++-- 3 files changed, 33 insertions(+), 5 deletions(-) diff --git a/dom/locales/en-US/chrome/dom/dom.properties b/dom/locales/en-US/chrome/dom/dom.properties index f0a6363af0..4ae891f327 100644 --- a/dom/locales/en-US/chrome/dom/dom.properties +++ b/dom/locales/en-US/chrome/dom/dom.properties @@ -224,7 +224,7 @@ ServiceWorkerScopePathMismatch=Failed to register a ServiceWorker: The path of t # LOCALIZATION NOTE: Do not translate "ServiceWorker". %1$S is a URL representing the scope of the ServiceWorker, %2$S is a stringified numeric HTTP status code like "404" and %3$S is a URL. ServiceWorkerRegisterNetworkError=Failed to register/update a ServiceWorker for scope ‘%1$S’: Load failed with status %2$S for script ‘%3$S’. # LOCALIZATION NOTE: Do not translate "ServiceWorker". %1$S is a URL representing the scope of the ServiceWorker, %2$S is a MIME Media Type like "text/plain" and %3$S is a URL. -ServiceWorkerRegisterMimeTypeError=Failed to register/update a ServiceWorker for scope ‘%1$S’: Bad Content-Type of ‘%2$S’ received for script ‘%3$S’. Must be ‘text/javascript’, ‘application/x-javascript’, or ‘application/javascript’. +ServiceWorkerRegisterMimeTypeError2=Failed to register/update a ServiceWorker for scope ‘%1$S’: Bad Content-Type of ‘%2$S’ received for script ‘%3$S’. Must be ‘text/javascript’, ‘application/x-javascript’, or ‘application/javascript’. # LOCALIZATION NOTE: Do not translate "ServiceWorker". %1$S is a URL representing the scope of the ServiceWorker. ServiceWorkerGraceTimeoutTermination=Terminating ServiceWorker for scope ‘%1$S’ with pending waitUntil/respondWith promises because of grace timeout. ExecCommandCutCopyDeniedNotInputDriven=document.execCommand(‘cut’/‘copy’) was denied because it was not called from inside a short running user-generated event handler. diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 0a9b7207e7..16f0dd1527 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -59,6 +59,7 @@ #include "mozilla/dom/ScriptLoader.h" #include "mozilla/dom/ScriptSettings.h" #include "mozilla/dom/SRILogHelper.h" +#include "mozilla/dom/workers/ServiceWorkerManager.h" #include "mozilla/UniquePtr.h" #include "Principal.h" #include "WorkerHolder.h" @@ -663,6 +664,34 @@ private: ScriptLoadInfo& loadInfo = mLoadInfos[aIndex]; nsCOMPtr channel = do_QueryInterface(aRequest); + + // Checking the MIME type is only required for ServiceWorkers' + // importScripts, per step 10 of https://w3c.github.io/ServiceWorker/#importscripts + // + // "Extract a MIME type from the response’s header list. If this MIME type + // (ignoring parameters) is not a JavaScript MIME type, return a network error." + if (mWorkerPrivate->IsServiceWorker()) { + nsAutoCString mimeType; + channel->GetContentType(mimeType); + + if (!nsContentUtils::IsJavascriptMIMEType(NS_ConvertUTF8toUTF16(mimeType))) { + const nsCString& scope = + mWorkerPrivate->ServiceWorkerScope(); + + ServiceWorkerManager::LocalizeAndReportToAllClients( + scope, "ServiceWorkerRegisterMimeTypeError2", + nsTArray { + NS_ConvertUTF8toUTF16(scope), + NS_ConvertUTF8toUTF16(mimeType), + loadInfo.mURL + } + ); + + channel->Cancel(NS_ERROR_DOM_NETWORK_ERR); + return NS_ERROR_DOM_NETWORK_ERR; + } + } + MOZ_ASSERT(channel == loadInfo.mChannel); // We synthesize the result code, but its never exposed to content. diff --git a/dom/workers/ServiceWorkerScriptCache.cpp b/dom/workers/ServiceWorkerScriptCache.cpp index 00e0446edf..c93ddd7c0e 100644 --- a/dom/workers/ServiceWorkerScriptCache.cpp +++ b/dom/workers/ServiceWorkerScriptCache.cpp @@ -801,12 +801,11 @@ CompareNetwork::OnStreamComplete(nsIStreamLoader* aLoader, nsISupports* aContext return rv; } - if (!mimeType.LowerCaseEqualsLiteral("text/javascript") && - !mimeType.LowerCaseEqualsLiteral("application/x-javascript") && - !mimeType.LowerCaseEqualsLiteral("application/javascript")) { + if (mimeType.IsEmpty() || + !nsContentUtils::IsJavascriptMIMEType(NS_ConvertUTF8toUTF16(mimeType))) { RefPtr registration = mManager->GetRegistration(); ServiceWorkerManager::LocalizeAndReportToAllClients( - registration->mScope, "ServiceWorkerRegisterMimeTypeError", + registration->mScope, "ServiceWorkerRegisterMimeTypeError2", nsTArray { NS_ConvertUTF8toUTF16(registration->mScope), NS_ConvertUTF8toUTF16(mimeType), mManager->URL() }); mManager->NetworkFinished(NS_ERROR_DOM_SECURITY_ERR); From 811bcbefd469cc1727a60250968a7a60708192e3 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 7 Jan 2024 08:29:30 -0600 Subject: [PATCH 16/25] Issue #2402 - Optionally strictly enforce the MIME type of scripts loaded by importScripts(). https://bugzilla.mozilla.org/show_bug.cgi?id=1514680 This is default on in Firefox 67 but Moonchild requested it be set off by default. --- .../en-US/chrome/security/security.properties | 2 + modules/libpref/init/all.js | 3 ++ netwerk/protocol/http/nsHttpChannel.cpp | 51 +++++++++++-------- 3 files changed, 34 insertions(+), 22 deletions(-) diff --git a/dom/locales/en-US/chrome/security/security.properties b/dom/locales/en-US/chrome/security/security.properties index 2be56fb9d3..988b6d8f1a 100644 --- a/dom/locales/en-US/chrome/security/security.properties +++ b/dom/locales/en-US/chrome/security/security.properties @@ -82,6 +82,8 @@ MimeTypeMismatch=The resource from “%1$S” was blocked due to MIME type misma XCTOHeaderValueMissing=X-Content-Type-Options header warning: value was “%1$S”; did you mean to send “nosniff”? BlockScriptWithWrongMimeType=Script from “%1$S” was blocked because of a disallowed MIME type. +# LOCALIZATION NOTE: Do not translate "importScripts()" +BlockImportScriptsWithWrongMimeType=Loading script from “%1$S” with importScripts() was blocked because of a disallowed MIME type. # LOCALIZATION NOTE: Do not translate "data: URI". BlockTopLevelDataURINavigation=Navigation to toplevel data: URI not allowed (Blocked loading of: “%1$S”) diff --git a/modules/libpref/init/all.js b/modules/libpref/init/all.js index 1f0ad89f71..fef4c449cd 100644 --- a/modules/libpref/init/all.js +++ b/modules/libpref/init/all.js @@ -2202,6 +2202,9 @@ pref("security.sri.enable", true); // Block scripts with wrong MIME type such as image/ or video/. pref("security.block_script_with_wrong_mime", true); +// Block scripts with wrong MIME type when loading via importScripts() in workers. +pref("security.block_importScripts_with_wrong_mime", false); + // Block images of wrong MIME for XCTO: nosniff. pref("security.xcto_nosniff_block_images", false); diff --git a/netwerk/protocol/http/nsHttpChannel.cpp b/netwerk/protocol/http/nsHttpChannel.cpp index 1bf57a0d3b..ea95740870 100644 --- a/netwerk/protocol/http/nsHttpChannel.cpp +++ b/netwerk/protocol/http/nsHttpChannel.cpp @@ -1160,35 +1160,42 @@ EnsureMIMEOfScript(nsIURI* aURI, nsHttpResponseHead* aResponseHead, nsILoadInfo* if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("text/plain"))) { // script load has type text/plain - return NS_OK; - } - - if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("text/xml"))) { + } else if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("text/xml"))) { // script load has type text/xml - return NS_OK; - } - - if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("application/octet-stream"))) { + } else if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("application/octet-stream"))) { // script load has type application/octet-stream - return NS_OK; - } - - if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("application/xml"))) { + } else if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("application/xml"))) { // script load has type application/xml - return NS_OK; - } - - if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("text/html"))) { + } else if (StringBeginsWith(contentType, NS_LITERAL_CSTRING("text/html"))) { // script load has type text/html - return NS_OK; - } - - if (contentType.IsEmpty()) { + } else if (contentType.IsEmpty()) { // script load has no type - return NS_OK; + } else { + // script load has unknown type + // We restrict importScripts() in worker code to JavaScript MIME types. + if (aLoadInfo->InternalContentPolicyType() == + nsIContentPolicy::TYPE_INTERNAL_WORKER_IMPORT_SCRIPTS) { + // Instead of consulting Preferences::GetBool() all the time we + // can cache the result to speed things up. + static bool sCachedBlockImportScriptsWithWrongMime = false; + static bool sIsInited = false; + if (!sIsInited) { + sIsInited = true; + Preferences::AddBoolVarCache( + &sCachedBlockImportScriptsWithWrongMime, + "security.block_importScripts_with_wrong_mime"); + } + + // Do not block the load if the feature is not enabled. + if (!sCachedBlockImportScriptsWithWrongMime) { + return NS_OK; + } + + ReportTypeBlocking(aURI, aLoadInfo, "BlockImportScriptsWithWrongMimeType"); + return NS_ERROR_CORRUPTED_CONTENT; + } } - // script load has unknown type return NS_OK; } From 9cd8aafeaac789501e134b3d7a2c32687614f87c Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 7 Jan 2024 11:48:56 -0600 Subject: [PATCH 17/25] Issue #2402 - CSP violation: blockedURI inline/eval. https://bugzilla.mozilla.org/show_bug.cgi?id=1418241 CSP: Blocked URI should be empty for inline violations. https://bugzilla.mozilla.org/show_bug.cgi?id=1236222 --- dom/security/nsCSPContext.cpp | 54 +++++++++++++++++------------------ dom/security/nsCSPContext.h | 8 +++--- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 3b74aea09c..5c52d0f201 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -489,7 +489,7 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, nsCOMPtr selfICString(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID)); if (selfICString) { - selfICString->SetData(nsDependentCString("self")); + selfICString->SetData(nsDependentCString("inline")); } nsCOMPtr selfISupports(do_QueryInterface(selfICString)); @@ -652,7 +652,16 @@ nsCSPContext::LogViolationDetails(uint16_t aViolationType, nsCOMPtr selfICString(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID)); if (selfICString) { - selfICString->SetData(nsDependentCString("self")); + if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL) { + selfICString->SetData(nsDependentCString("eval")); + } else if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT || + aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_STYLE) { + selfICString->SetData(nsDependentCString("inline")); + } else { + // All the other types should have a URL, but just in case, let's use + // 'self' here. + selfICString->SetData(nsDependentCString("self")); + } } nsCOMPtr selfISupports(do_QueryInterface(selfICString)); @@ -827,7 +836,8 @@ StripURIForReporting(nsIURI* aURI, nsresult nsCSPContext::GatherSecurityPolicyViolationEventData( - nsISupports* aBlockedContentSource, + nsIURI* aBlockedURI, + const nsACString& aBlockedString, nsIURI* aOriginalURI, nsAString& aViolatedDirective, uint32_t aViolatedPolicyIndex, @@ -861,24 +871,12 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( aViolationEventInit.mReferrer = mReferrer; // blocked-uri - if (aBlockedContentSource) { + if (aBlockedURI) { nsAutoCString reportBlockedURI; - nsCOMPtr uri = do_QueryInterface(aBlockedContentSource); - // could be a string or URI - if (uri) { - StripURIForReporting(uri, mSelfURI, reportBlockedURI); - } else { - nsCOMPtr cstr = do_QueryInterface(aBlockedContentSource); - if (cstr) { - cstr->GetData(reportBlockedURI); - } - } - if (reportBlockedURI.IsEmpty()) { - // this can happen for frame-ancestors violation where the violating - // ancestor is cross-origin. - NS_WARNING("No blocked URI (null aBlockedContentSource) for CSP violation report."); - } + StripURIForReporting(aBlockedURI, mSelfURI, reportBlockedURI); aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI); + } else { + aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(aBlockedString); } // effective-directive @@ -1198,8 +1196,16 @@ class CSPReportSenderRunnable final : public Runnable // 0) prepare violation data mozilla::dom::SecurityPolicyViolationEventInit init; + // mBlockedContentSource could be a URI or a string. + nsCOMPtr blockedURI = do_QueryInterface(mBlockedContentSource); + // if mBlockedContentSource is not a URI, it could be a string + nsCOMPtr blockedICString = do_QueryInterface(mBlockedContentSource); + nsAutoCString blockedDataStr; + if (blockedICString) { + blockedICString->GetData(blockedDataStr); + } rv = mCSPContext->GatherSecurityPolicyViolationEventData( - mBlockedContentSource, mOriginalURI, + blockedURI, blockedDataStr, mOriginalURI, mViolatedDirective, mViolatedPolicyIndex, mSourceFile, mScriptSample, mLineNum, init); @@ -1217,12 +1223,6 @@ class CSPReportSenderRunnable final : public Runnable mCSPContext->SendReports(init, mViolatedPolicyIndex); // 3) log to console (one per policy violation) - // mBlockedContentSource could be a URI or a string. - nsCOMPtr blockedURI = do_QueryInterface(mBlockedContentSource); - // if mBlockedContentSource is not a URI, it could be a string - nsCOMPtr blockedString = do_QueryInterface(mBlockedContentSource); - - nsCString blockedDataStr; if (blockedURI) { blockedURI->GetSpec(blockedDataStr); @@ -1232,8 +1232,6 @@ class CSPReportSenderRunnable final : public Runnable blockedDataStr.Truncate(40); blockedDataStr.AppendASCII("..."); } - } else if (blockedString) { - blockedString->GetData(blockedDataStr); } if (blockedDataStr.Length() > 0) { diff --git a/dom/security/nsCSPContext.h b/dom/security/nsCSPContext.h index 8a283c1a10..1d3122af80 100644 --- a/dom/security/nsCSPContext.h +++ b/dom/security/nsCSPContext.h @@ -61,9 +61,8 @@ class nsCSPContext : public nsIContentSecurityPolicy /** * Construct SecurityPolicyViolationEventInit structure. * - * @param aBlockedContentSource - * Either a CSP Source (like 'self', as string) or nsIURI: the source - * of the violation. + * @param aBlockedURI + * A nsIURI: the source of the violation. * @param aOriginalUri * The original URI if the blocked content is a redirect, else null * @param aViolatedDirective @@ -78,7 +77,8 @@ class nsCSPContext : public nsIContentSecurityPolicy * The output */ nsresult GatherSecurityPolicyViolationEventData( - nsISupports* aBlockedContentSource, + nsIURI* aBlockedURI, + const nsACString& aBlockedString, nsIURI* aOriginalURI, nsAString& aViolatedDirective, uint32_t aViolatedPolicyIndex, From 0e0ec8cb207a78917979adaddcf7a5a31f29b906 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Sun, 7 Jan 2024 11:59:24 -0600 Subject: [PATCH 18/25] Issue #2402 - CSP should throw EvalError when blocking eval() and Function(). https://bugzilla.mozilla.org/show_bug.cgi?id=1473024 https://bugzilla.mozilla.org/show_bug.cgi?id=1490165 --- js/src/js.msg | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/js.msg b/js/src/js.msg index b8aa77902d..1fa7a65661 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -150,8 +150,8 @@ MSG_DEF(JSMSG_PARAMETER_AFTER_REST, 0, JSEXN_SYNTAXERR, "parameter after rest MSG_DEF(JSMSG_TOO_MANY_ARGUMENTS, 0, JSEXN_RANGEERR, "too many arguments provided for a function call") // CSP -MSG_DEF(JSMSG_CSP_BLOCKED_EVAL, 0, JSEXN_ERR, "call to eval() blocked by CSP") -MSG_DEF(JSMSG_CSP_BLOCKED_FUNCTION, 0, JSEXN_ERR, "call to Function() blocked by CSP") +MSG_DEF(JSMSG_CSP_BLOCKED_EVAL, 0, JSEXN_EVALERR, "call to eval() blocked by CSP") +MSG_DEF(JSMSG_CSP_BLOCKED_FUNCTION, 0, JSEXN_EVALERR, "call to Function() blocked by CSP") // Wrappers MSG_DEF(JSMSG_ACCESSOR_DEF_DENIED, 1, JSEXN_ERR, "Permission denied to define accessor property {0}") From e74612e23e6dbbdaf1beb29161e873291ba1220c Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 8 Jan 2024 03:22:44 -0600 Subject: [PATCH 19/25] Issue #2402 - Workers.setTimeout/setInterval must handle CSP rejections. https://bugzilla.mozilla.org/show_bug.cgi?id=1490165 Has some namespace issues adding the files, so differs slightly. --- dom/base/nsJSTimeoutHandler.cpp | 77 ++++---------- dom/security/CSPEvalChecker.cpp | 183 ++++++++++++++++++++++++++++++++ dom/security/CSPEvalChecker.h | 37 +++++++ dom/security/moz.build | 3 + dom/workers/Queue.h | 2 +- dom/workers/ScriptLoader.cpp | 2 +- dom/workers/WorkerPrivate.h | 6 +- dom/workers/WorkerRunnable.h | 4 +- dom/workers/WorkerScope.cpp | 16 ++- dom/workers/moz.build | 5 + 10 files changed, 270 insertions(+), 65 deletions(-) create mode 100644 dom/security/CSPEvalChecker.cpp create mode 100644 dom/security/CSPEvalChecker.h diff --git a/dom/base/nsJSTimeoutHandler.cpp b/dom/base/nsJSTimeoutHandler.cpp index 5c836694ae..6e8c6155c4 100644 --- a/dom/base/nsJSTimeoutHandler.cpp +++ b/dom/base/nsJSTimeoutHandler.cpp @@ -9,6 +9,7 @@ #include "mozilla/Function.h" #include "mozilla/Likely.h" #include "mozilla/Maybe.h" +#include "mozilla/dom/CSPEvalChecker.h" #include "mozilla/dom/FunctionBinding.h" #include "mozilla/dom/ModuleScript.h" #include "nsAXPCNativeCallContext.h" @@ -52,7 +53,8 @@ public: Function& aFunction, nsTArray>&& aArguments); nsJSScriptTimeoutHandler(JSContext* aCx, WorkerPrivate* aWorkerPrivate, - const nsAString& aExpression); + const nsAString& aExpression, bool* aAllowEval, + ErrorResult& aRv); virtual const nsAString& GetHandlerText() override; @@ -182,54 +184,6 @@ NS_INTERFACE_MAP_END NS_IMPL_CYCLE_COLLECTING_ADDREF(nsJSScriptTimeoutHandler) NS_IMPL_CYCLE_COLLECTING_RELEASE(nsJSScriptTimeoutHandler) -static bool -CheckCSPForEval(JSContext* aCx, nsGlobalWindow* aWindow, ErrorResult& aError) -{ - // if CSP is enabled, and setTimeout/setInterval was called with a string, - // disable the registration and log an error - nsCOMPtr doc = aWindow->GetExtantDoc(); - if (!doc) { - // if there's no document, we don't have to do anything. - return true; - } - - nsCOMPtr csp; - aError = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp)); - if (aError.Failed()) { - return false; - } - - if (!csp) { - return true; - } - - bool allowsEval = true; - bool reportViolation = false; - aError = csp->GetAllowsEval(&reportViolation, &allowsEval); - if (aError.Failed()) { - return false; - } - - if (reportViolation) { - // TODO : need actual script sample in violation report. - NS_NAMED_LITERAL_STRING(scriptSample, - "call to eval() or related function blocked by CSP"); - - // Get the calling location. - uint32_t lineNum = 0; - nsAutoString fileNameString; - if (!nsJSUtils::GetCallingLocation(aCx, fileNameString, &lineNum)) { - fileNameString.AssignLiteral("unknown"); - } - - csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL, - fileNameString, scriptSample, lineNum, - EmptyString(), EmptyString()); - } - - return allowsEval; -} - nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler() : mLineNo(0) , mColumn(0) @@ -273,8 +227,9 @@ nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler(JSContext* aCx, return; } - *aAllowEval = CheckCSPForEval(aCx, aWindow, aError); - if (aError.Failed() || !*aAllowEval) { + aError = CSPEvalChecker::CheckForWindow(aCx, aWindow, aExpression, + aAllowEval); + if (NS_WARN_IF(aError.Failed()) || !*aAllowEval) { return; } @@ -297,7 +252,9 @@ nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler(JSContext* aCx, nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler(JSContext* aCx, WorkerPrivate* aWorkerPrivate, - const nsAString& aExpression) + const nsAString& aExpression, + bool* aAllowEval, + ErrorResult& aError) : mLineNo(0) , mColumn(0) , mExpr(aExpression) @@ -305,6 +262,12 @@ nsJSScriptTimeoutHandler::nsJSScriptTimeoutHandler(JSContext* aCx, MOZ_ASSERT(aWorkerPrivate); aWorkerPrivate->AssertIsOnWorkerThread(); + aError = CSPEvalChecker::CheckForWorker(aCx, aWorkerPrivate, aExpression, + aAllowEval); + if (NS_WARN_IF(aError.Failed()) || !*aAllowEval) { + return; + } + Init(aCx); } @@ -399,9 +362,15 @@ NS_CreateJSTimeoutHandler(JSContext *aCx, WorkerPrivate* aWorkerPrivate, already_AddRefed NS_CreateJSTimeoutHandler(JSContext* aCx, WorkerPrivate* aWorkerPrivate, - const nsAString& aExpression) + const nsAString& aExpression, ErrorResult& aRv) { + bool allowEval = false; RefPtr handler = - new nsJSScriptTimeoutHandler(aCx, aWorkerPrivate, aExpression); + new nsJSScriptTimeoutHandler(aCx, aWorkerPrivate, aExpression, &allowEval, + aRv); + if (aRv.Failed() || !allowEval) { + return nullptr; + } + return handler.forget(); } diff --git a/dom/security/CSPEvalChecker.cpp b/dom/security/CSPEvalChecker.cpp new file mode 100644 index 0000000000..42b5610c7d --- /dev/null +++ b/dom/security/CSPEvalChecker.cpp @@ -0,0 +1,183 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "mozilla/dom/CSPEvalChecker.h" +#include "mozilla/dom/WorkerPrivate.h" +#include "mozilla/dom/WorkerRunnable.h" +#include "mozilla/ErrorResult.h" +#include "nsGlobalWindow.h" +#include "nsIDocument.h" +#include "nsCOMPtr.h" +#include "nsJSUtils.h" + +using namespace mozilla; +using namespace mozilla::dom; + +namespace { + +nsresult +CheckInternal(nsIContentSecurityPolicy* aCSP, + const nsAString& aExpression, + const nsAString& aFileNameString, + uint32_t aLineNum, + uint32_t aColumnNum, + bool* aAllowed) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(aAllowed); + + // The value is set at any "return", but better to have a default value here. + *aAllowed = false; + + if (!aCSP) { + *aAllowed = true; + return NS_OK; + } + + bool reportViolation = false; + nsresult rv = aCSP->GetAllowsEval(&reportViolation, aAllowed); + if (NS_WARN_IF(NS_FAILED(rv))) { + *aAllowed = false; + return rv; + } + + if (reportViolation) { + aCSP->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL, + aFileNameString, aExpression, aLineNum, + EmptyString(), EmptyString()); + } + + return NS_OK; +} + +class WorkerCSPCheckRunnable final : public WorkerMainThreadRunnable +{ +public: + WorkerCSPCheckRunnable(WorkerPrivate* aWorkerPrivate, + const nsAString& aExpression, + const nsAString& aFileNameString, + uint32_t aLineNum, + uint32_t aColumnNum) + : WorkerMainThreadRunnable(aWorkerPrivate, + NS_LITERAL_CSTRING("CSP Eval Check")) + , mExpression(aExpression) + , mFileNameString(aFileNameString) + , mLineNum(aLineNum) + , mColumnNum(aColumnNum) + , mEvalAllowed(false) + {} + + bool + MainThreadRun() override + { + mResult = CheckInternal(mWorkerPrivate->GetCSP(), mExpression, + mFileNameString, mLineNum, mColumnNum, + &mEvalAllowed); + return true; + } + + nsresult + GetResult(bool* aAllowed) + { + MOZ_ASSERT(aAllowed); + *aAllowed = mEvalAllowed; + return mResult; + } + +private: + const nsString mExpression; + const nsString mFileNameString; + const uint32_t mLineNum; + const uint32_t mColumnNum; + bool mEvalAllowed; + nsresult mResult; +}; + +} // anonymous + +/* static */ nsresult +CSPEvalChecker::CheckForWindow(JSContext* aCx, nsGlobalWindow* aWindow, + const nsAString& aExpression, bool* aAllowEval) +{ + MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(aWindow); + MOZ_ASSERT(aAllowEval); + + // The value is set at any "return", but better to have a default value here. + *aAllowEval = false; + + // if CSP is enabled, and setTimeout/setInterval was called with a string, + // disable the registration and log an error + nsCOMPtr doc = aWindow->GetExtantDoc(); + if (!doc) { + // if there's no document, we don't have to do anything. + *aAllowEval = true; + return NS_OK; + } + + nsCOMPtr csp; + nsresult rv = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp)); + if (NS_WARN_IF(NS_FAILED(rv))) { + *aAllowEval = false; + return rv; + } + + // Get the calling location. + uint32_t lineNum = 0; + uint32_t columnNum = 0; + nsAutoString fileNameString; + if (!nsJSUtils::GetCallingLocation(aCx, fileNameString, &lineNum, + &columnNum)) { + fileNameString.AssignLiteral("unknown"); + } + + rv = CheckInternal(csp, aExpression, fileNameString, lineNum, columnNum, + aAllowEval); + if (NS_WARN_IF(NS_FAILED(rv))) { + *aAllowEval = false; + return rv; + } + + return NS_OK; +} + +/* static */ nsresult +CSPEvalChecker::CheckForWorker(JSContext* aCx, WorkerPrivate* aWorkerPrivate, + const nsAString& aExpression, bool* aAllowEval) +{ + MOZ_ASSERT(aWorkerPrivate); + aWorkerPrivate->AssertIsOnWorkerThread(); + MOZ_ASSERT(aAllowEval); + + // The value is set at any "return", but better to have a default value here. + *aAllowEval = false; + + // Get the calling location. + uint32_t lineNum = 0; + uint32_t columnNum = 0; + nsAutoString fileNameString; + if (!nsJSUtils::GetCallingLocation(aCx, fileNameString, &lineNum, + &columnNum)) { + fileNameString.AssignLiteral("unknown"); + } + + RefPtr r = + new WorkerCSPCheckRunnable(aWorkerPrivate, aExpression, fileNameString, + lineNum, columnNum); + ErrorResult error; + r->Dispatch(Canceling, error); + if (NS_WARN_IF(error.Failed())) { + *aAllowEval = false; + return error.StealNSResult(); + } + + nsresult rv = r->GetResult(aAllowEval); + if (NS_WARN_IF(NS_FAILED(rv))) { + *aAllowEval = false; + return rv; + } + + return NS_OK; +} diff --git a/dom/security/CSPEvalChecker.h b/dom/security/CSPEvalChecker.h new file mode 100644 index 0000000000..5a73d7652f --- /dev/null +++ b/dom/security/CSPEvalChecker.h @@ -0,0 +1,37 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#ifndef mozilla_dom_CSPEvalChecker_h +#define mozilla_dom_CSPEvalChecker_h + +#include "nsString.h" + +struct JSContext; +class nsGlobalWindow; + +namespace mozilla { +namespace dom { +namespace workers { +class WorkerPrivate; +} + +using namespace mozilla::dom::workers; + +class CSPEvalChecker final +{ +public: + static nsresult + CheckForWindow(JSContext* aCx, nsGlobalWindow* aWindow, + const nsAString& aExpression, bool* aAllowEval); + + static nsresult + CheckForWorker(JSContext* aCx, WorkerPrivate* aWorkerPrivate, + const nsAString& aExpression, bool* aAllowEval); +}; + +} // dom namespace +} // mozilla namespace + +#endif // mozilla_dom_CSPEvalChecker_h diff --git a/dom/security/moz.build b/dom/security/moz.build index 3f690ea498..224b7a4a9a 100644 --- a/dom/security/moz.build +++ b/dom/security/moz.build @@ -7,6 +7,7 @@ TEST_DIRS += ['test'] EXPORTS.mozilla.dom += [ 'ContentVerifier.h', + 'CSPEvalChecker.h', 'nsContentSecurityManager.h', 'nsCSPContext.h', 'nsCSPService.h', @@ -24,6 +25,7 @@ EXPORTS += [ UNIFIED_SOURCES += [ 'ContentVerifier.cpp', + 'CSPEvalChecker.cpp', 'nsContentSecurityManager.cpp', 'nsCSPContext.cpp', 'nsCSPParser.cpp', @@ -39,6 +41,7 @@ include('/ipc/chromium/chromium-config.mozbuild') FINAL_LIBRARY = 'xul' LOCAL_INCLUDES += [ '/caps', + '/dom/base', '/netwerk/base', ] diff --git a/dom/workers/Queue.h b/dom/workers/Queue.h index aa673f587e..b6513acdb7 100644 --- a/dom/workers/Queue.h +++ b/dom/workers/Queue.h @@ -6,7 +6,7 @@ #ifndef mozilla_dom_workers_queue_h__ #define mozilla_dom_workers_queue_h__ -#include "Workers.h" +#include "mozilla/dom/workers/Workers.h" #include "mozilla/Mutex.h" #include "nsTArray.h" diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 16f0dd1527..646182bfd3 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1163,7 +1163,7 @@ private: ("Scriptloader::Load, SRI required but not supported in workers")); nsCOMPtr wcsp; chanLoadInfo->LoadingPrincipal()->GetCsp(getter_AddRefs(wcsp)); - MOZ_ASSERT(wcsp, "We sould have a CSP for the worker here"); + MOZ_ASSERT(wcsp, "We should have a CSP for the worker here"); if (wcsp) { wcsp->LogViolationDetails( nsIContentSecurityPolicy::VIOLATION_TYPE_REQUIRE_SRI_FOR_SCRIPT, diff --git a/dom/workers/WorkerPrivate.h b/dom/workers/WorkerPrivate.h index dda81beb57..0ca8766438 100644 --- a/dom/workers/WorkerPrivate.h +++ b/dom/workers/WorkerPrivate.h @@ -6,7 +6,7 @@ #ifndef mozilla_dom_workers_workerprivate_h__ #define mozilla_dom_workers_workerprivate_h__ -#include "Workers.h" +#include "mozilla/dom/workers/Workers.h" #include "js/CharacterEncoding.h" #include "nsIContentPolicy.h" @@ -34,8 +34,8 @@ #include "nsThreadUtils.h" #include "nsTObserverArray.h" -#include "Queue.h" -#include "WorkerHolder.h" +#include "mozilla/dom/workerinternals/Queue.h" +#include "mozilla/dom/workers/bindings/WorkerHolder.h" #ifdef XP_WIN #undef PostMessage diff --git a/dom/workers/WorkerRunnable.h b/dom/workers/WorkerRunnable.h index 7484ba6855..05fe31df8b 100644 --- a/dom/workers/WorkerRunnable.h +++ b/dom/workers/WorkerRunnable.h @@ -6,14 +6,14 @@ #ifndef mozilla_dom_workers_workerrunnable_h__ #define mozilla_dom_workers_workerrunnable_h__ -#include "Workers.h" +#include "mozilla/dom/workers/Workers.h" #include "nsICancelableRunnable.h" #include "mozilla/Atomics.h" #include "nsISupportsImpl.h" #include "nsThreadUtils.h" /* nsRunnable */ -#include "WorkerHolder.h" +#include "mozilla/dom/workers/bindings/WorkerHolder.h" struct JSContext; class nsIEventTarget; diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index df75ff887a..ef6abc25e2 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -58,7 +58,8 @@ NS_CreateJSTimeoutHandler(JSContext* aCx, extern already_AddRefed NS_CreateJSTimeoutHandler(JSContext* aCx, mozilla::dom::workers::WorkerPrivate* aWorkerPrivate, - const nsAString& aExpression); + const nsAString& aExpression, + mozilla::ErrorResult& aRv); using namespace mozilla; using namespace mozilla::dom; @@ -268,7 +269,7 @@ WorkerGlobalScope::SetTimeout(JSContext* aCx, nsCOMPtr handler = NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler, aArguments, aRv); - if (NS_WARN_IF(aRv.Failed())) { + if (!handler) { return 0; } @@ -285,7 +286,11 @@ WorkerGlobalScope::SetTimeout(JSContext* aCx, mWorkerPrivate->AssertIsOnWorkerThread(); nsCOMPtr handler = - NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler); + NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler, aRv); + if (!handler) { + return 0; + } + return mWorkerPrivate->SetTimeout(aCx, handler, aTimeout, false, aRv); } @@ -326,7 +331,10 @@ WorkerGlobalScope::SetInterval(JSContext* aCx, Sequence dummy; nsCOMPtr handler = - NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler); + NS_CreateJSTimeoutHandler(aCx, mWorkerPrivate, aHandler, aRv); + if (NS_WARN_IF(aRv.Failed())) { + return 0; + } return mWorkerPrivate->SetTimeout(aCx, handler, aTimeout, true, aRv); } diff --git a/dom/workers/moz.build b/dom/workers/moz.build index 5421d65a8d..e3fe845f9e 100644 --- a/dom/workers/moz.build +++ b/dom/workers/moz.build @@ -19,6 +19,11 @@ EXPORTS.mozilla.dom += [ 'WorkerScope.h', ] +# Private stuff. +EXPORTS.mozilla.dom.workerinternals += [ + 'Queue.h', +] + EXPORTS.mozilla.dom.workers += [ 'RuntimeService.h', 'ServiceWorkerCommon.h', From 5b068f37264449b58a3c4e78a631462ca9eb1679 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 8 Jan 2024 07:46:11 -0600 Subject: [PATCH 20/25] Issue #2402 - CSP Violation events should have the correct sample for inline contexts. https://bugzilla.mozilla.org/show_bug.cgi?id=1473587 Add preference to increase max length of CSP report source sample. https://bugzilla.mozilla.org/show_bug.cgi?id=1415352 Return valid columnNumber value in CSP violation events. https://bugzilla.mozilla.org/show_bug.cgi?id=1418246 --- caps/nsScriptSecurityManager.cpp | 23 ++++- caps/nsScriptSecurityManager.h | 2 +- dom/base/nsDocument.cpp | 1 + dom/base/nsIStyleSheetLinkingElement.h | 13 +++ dom/base/nsStyleLinkElement.cpp | 17 +++- dom/base/nsStyleLinkElement.h | 3 + dom/base/nsStyledElement.cpp | 2 +- dom/events/EventListenerManager.cpp | 28 +++--- .../security/nsIContentSecurityPolicy.idl | 8 +- dom/jsurl/nsJSProtocolHandler.cpp | 1 + dom/script/ScriptLoader.cpp | 4 +- dom/script/nsIScriptElement.h | 16 ++++ dom/security/CSPEvalChecker.cpp | 2 +- dom/security/nsCSPContext.cpp | 92 ++++++++++++++----- dom/security/nsCSPContext.h | 15 ++- dom/smil/nsSMILCSSValueType.cpp | 2 +- dom/webidl/CSPReport.webidl | 1 + dom/workers/RuntimeService.cpp | 35 +++++-- dom/workers/ScriptLoader.cpp | 2 +- dom/xbl/nsXBLContentSink.cpp | 10 +- dom/xbl/nsXBLContentSink.h | 6 +- dom/xml/nsXMLContentSink.cpp | 13 ++- dom/xml/nsXMLContentSink.h | 5 +- dom/xml/nsXMLFragmentContentSink.cpp | 7 +- dom/xslt/xslt/txMozillaStylesheetCompiler.cpp | 3 +- dom/xul/nsXULContentSink.cpp | 7 +- js/public/Principals.h | 4 +- js/src/builtin/Eval.cpp | 5 +- js/src/jsfun.cpp | 15 +-- js/src/vm/GlobalObject.cpp | 14 ++- js/src/vm/GlobalObject.h | 3 +- layout/style/Loader.cpp | 2 +- layout/style/nsStyleUtil.cpp | 3 +- layout/style/nsStyleUtil.h | 4 + parser/htmlparser/nsExpatDriver.cpp | 3 +- parser/htmlparser/nsIExpatSink.idl | 4 +- parser/xml/nsSAXXMLReader.cpp | 3 +- rdf/base/nsRDFContentSink.cpp | 5 +- 38 files changed, 277 insertions(+), 106 deletions(-) diff --git a/caps/nsScriptSecurityManager.cpp b/caps/nsScriptSecurityManager.cpp index 8f36fa9171..e7dedba8fa 100644 --- a/caps/nsScriptSecurityManager.cpp +++ b/caps/nsScriptSecurityManager.cpp @@ -535,7 +535,8 @@ NS_IMPL_ISUPPORTS(nsScriptSecurityManager, ///////////////// Security Checks ///////////////// bool -nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx) +nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx, + JS::HandleValue aValue) { MOZ_ASSERT(cx == nsContentUtils::GetCurrentJSContext()); nsCOMPtr subjectPrincipal = nsContentUtils::SubjectPrincipal(); @@ -558,12 +559,23 @@ nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx) } if (reportViolation) { - nsAutoString fileName; - unsigned lineNum = 0; - NS_NAMED_LITERAL_STRING(scriptSample, "call to eval() or related function blocked by CSP"); + JS::Rooted jsString(cx, JS::ToString(cx, aValue)); + if (NS_WARN_IF(!jsString)) { + JS_ClearPendingException(cx); + return false; + } + + nsAutoJSString scriptSample; + if (NS_WARN_IF(!scriptSample.init(cx, jsString))) { + JS_ClearPendingException(cx); + return false; + } JS::AutoFilename scriptFilename; - if (JS::DescribeScriptedCaller(cx, &scriptFilename, &lineNum)) { + nsAutoString fileName; + unsigned lineNum = 0; + unsigned columnNum = 0; + if (JS::DescribeScriptedCaller(cx, &scriptFilename, &lineNum, &columnNum)) { if (const char *file = scriptFilename.get()) { CopyUTF8toUTF16(nsDependentCString(file), fileName); } @@ -574,6 +586,7 @@ nsScriptSecurityManager::ContentSecurityPolicyPermitsJSAction(JSContext *cx) fileName, scriptSample, lineNum, + columnNum, EmptyString(), EmptyString()); } diff --git a/caps/nsScriptSecurityManager.h b/caps/nsScriptSecurityManager.h index c5a1e5cd28..b1953291db 100644 --- a/caps/nsScriptSecurityManager.h +++ b/caps/nsScriptSecurityManager.h @@ -91,7 +91,7 @@ private: // Decides, based on CSP, whether or not eval() and stuff can be executed. static bool - ContentSecurityPolicyPermitsJSAction(JSContext *cx); + ContentSecurityPolicyPermitsJSAction(JSContext *cx, JS::HandleValue aValue); static bool JSPrincipalsSubsume(JSPrincipals *first, JSPrincipals *second); diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 4d3fc70b83..ab9f2419a1 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -11910,6 +11910,7 @@ nsIDocument::InlineScriptAllowedByCSP() true, // aParserCreated EmptyString(), // FIXME get script sample (bug 1314567) 0, // aLineNumber + 0, // aColumnNumber &allowsInlineScript); NS_ENSURE_SUCCESS(rv, true); } diff --git a/dom/base/nsIStyleSheetLinkingElement.h b/dom/base/nsIStyleSheetLinkingElement.h index 614005640b..0bf3b5827f 100644 --- a/dom/base/nsIStyleSheetLinkingElement.h +++ b/dom/base/nsIStyleSheetLinkingElement.h @@ -101,6 +101,19 @@ public: * was set */ virtual uint32_t GetLineNumber() = 0; + + // This doesn't entirely belong here since they only make sense for + // some types of linking elements, but it's a better place than + // anywhere else. + virtual void SetColumnNumber(uint32_t aColumnNumber) = 0; + + /** + * Get the column number, as previously set by SetColumnNumber. + * + * @return the column number of this element; or 1 if no column number + * was set + */ + virtual uint32_t GetColumnNumber() = 0; }; NS_DEFINE_STATIC_IID_ACCESSOR(nsIStyleSheetLinkingElement, diff --git a/dom/base/nsStyleLinkElement.cpp b/dom/base/nsStyleLinkElement.cpp index 7ea7fce40c..0fbdbd5860 100644 --- a/dom/base/nsStyleLinkElement.cpp +++ b/dom/base/nsStyleLinkElement.cpp @@ -40,6 +40,7 @@ nsStyleLinkElement::nsStyleLinkElement() : mDontLoadStyle(false) , mUpdatesEnabled(true) , mLineNumber(1) + , mColumnNumber(1) { } @@ -127,6 +128,18 @@ nsStyleLinkElement::GetLineNumber() return mLineNumber; } +/* virtual */ void +nsStyleLinkElement::SetColumnNumber(uint32_t aColumnNumber) +{ + mColumnNumber = aColumnNumber; +} + +/* virtual */ uint32_t +nsStyleLinkElement::GetColumnNumber() +{ + return mColumnNumber; +} + /* static */ bool nsStyleLinkElement::IsImportEnabled() { @@ -412,8 +425,10 @@ nsStyleLinkElement::DoUpdateStyleSheet(nsIDocument* aOldDocument, if (!nsStyleUtil::CSPAllowsInlineStyle(thisContent, thisContent->NodePrincipal(), doc->GetDocumentURI(), - mLineNumber, text, &rv)) + mLineNumber, mColumnNumber, text, + &rv)) { return rv; + } // Parse the style sheet. rv = doc->CSSLoader()-> diff --git a/dom/base/nsStyleLinkElement.h b/dom/base/nsStyleLinkElement.h index a0664106aa..79ac6abbaa 100644 --- a/dom/base/nsStyleLinkElement.h +++ b/dom/base/nsStyleLinkElement.h @@ -54,6 +54,8 @@ public: virtual void OverrideBaseURI(nsIURI* aNewBaseURI) override; virtual void SetLineNumber(uint32_t aLineNumber) override; virtual uint32_t GetLineNumber() override; + void SetColumnNumber(uint32_t aColumnNumber) override; + uint32_t GetColumnNumber() override; enum RelValue { ePREFETCH = 0x00000001, @@ -140,6 +142,7 @@ protected: bool mDontLoadStyle; bool mUpdatesEnabled; uint32_t mLineNumber; + uint32_t mColumnNumber; }; #endif /* nsStyleLinkElement_h___ */ diff --git a/dom/base/nsStyledElement.cpp b/dom/base/nsStyledElement.cpp index 42b632e71a..b9331c076d 100644 --- a/dom/base/nsStyledElement.cpp +++ b/dom/base/nsStyledElement.cpp @@ -191,7 +191,7 @@ nsStyledElement::ParseStyleAttribute(const nsAString& aValue, if (!isNativeAnon && !nsStyleUtil::CSPAllowsInlineStyle(nullptr, NodePrincipal(), - doc->GetDocumentURI(), 0, aValue, + doc->GetDocumentURI(), 0, 0, aValue, nullptr)) return; diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 7d68394850..b2e5a19854 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -786,28 +786,22 @@ EventListenerManager::SetEventHandler(nsIAtom* aName, rv = doc->NodePrincipal()->GetCsp(getter_AddRefs(csp)); NS_ENSURE_SUCCESS(rv, rv); - if (csp) { - // let's generate a script sample and pass it as aContent, - // it will not match the hash, but allows us to pass - // the script sample in aContent. - nsAutoString scriptSample, attr, tagName(NS_LITERAL_STRING("UNKNOWN")); - aName->ToString(attr); - nsCOMPtr domNode(do_QueryInterface(mTarget)); - if (domNode) { - domNode->GetNodeName(tagName); - } - // build a "script sample" based on what we know about this element - scriptSample.Assign(attr); - scriptSample.AppendLiteral(" attribute on "); - scriptSample.Append(tagName); - scriptSample.AppendLiteral(" element"); + unsigned lineNum = 0; + unsigned columnNum = 0; + JSContext* cx = nsContentUtils::GetCurrentJSContext(); + if (cx && !JS::DescribeScriptedCaller(cx, nullptr, &lineNum, &columnNum)) { + JS_ClearPendingException(cx); + } + + if (csp) { bool allowsInlineScript = true; rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, EmptyString(), // aNonce true, // aParserCreated (true because attribute event handler) - scriptSample, - 0, // aLineNumber + aBody, + lineNum, // aLineNumber + columnNum, // aColumnNumber &allowsInlineScript); NS_ENSURE_SUCCESS(rv, rv); diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl index a8b508ce7e..bdcbf908bf 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl @@ -140,6 +140,8 @@ interface nsIContentSecurityPolicy : nsISerializable * (and compare to the hashes listed in the policy) * @param aLineNumber The line number of the inline resource * (used for reporting) + * @param aColumnNumber The column number of the inline resource + * (used for reporting) * @return * Whether or not the effects of the inline style should be allowed * (block the rules if false). @@ -148,7 +150,8 @@ interface nsIContentSecurityPolicy : nsISerializable in AString aNonce, in boolean aParserCreated, in AString aContent, - in unsigned long aLineNumber); + in unsigned long aLineNumber, + in unsigned long aColumnNumber); /** * whether this policy allows eval and eval-like functions @@ -188,6 +191,8 @@ interface nsIContentSecurityPolicy : nsISerializable * sample of the violating content (to aid debugging) * @param lineNum * source line number of the violation (if available) + * @param columnNum + * source column number of the violation (if available) * @param aNonce * (optional) If this is a nonce violation, include the nonce so we can * recheck to determine which policies were violated and send the @@ -202,6 +207,7 @@ interface nsIContentSecurityPolicy : nsISerializable in AString sourceFile, in AString scriptSample, in int32_t lineNum, + in int32_t columnNum, [optional] in AString nonce, [optional] in AString content); diff --git a/dom/jsurl/nsJSProtocolHandler.cpp b/dom/jsurl/nsJSProtocolHandler.cpp index fe3c12b76a..9a9541164f 100644 --- a/dom/jsurl/nsJSProtocolHandler.cpp +++ b/dom/jsurl/nsJSProtocolHandler.cpp @@ -185,6 +185,7 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel, true, // aParserCreated EmptyString(), // aContent 0, // aLineNumber + 0, // aColumnNumber &allowsInlineScript); //return early if inline scripts are not allowed diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index c07c4b184f..6c211d53f2 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -1474,6 +1474,7 @@ CSPAllowsInlineScript(nsIScriptElement *aElement, nsIDocument *aDocument) rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, nonce, parserCreated, scriptText, aElement->GetScriptLineNumber(), + aElement->GetScriptColumnNumber(), &allowInlineScript); return allowInlineScript; } @@ -2695,10 +2696,11 @@ ScriptLoader::VerifySRI(ScriptLoadRequest* aRequest, nsAutoCString violationURISpec; mDocument->GetDocumentURI()->GetAsciiSpec(violationURISpec); uint32_t lineNo = aRequest->Element() ? aRequest->Element()->GetScriptLineNumber() : 0; + uint32_t columnNo = aRequest->Element() ? aRequest->Element()->GetScriptColumnNumber() : 0; csp->LogViolationDetails( nsIContentSecurityPolicy::VIOLATION_TYPE_REQUIRE_SRI_FOR_SCRIPT, NS_ConvertUTF8toUTF16(violationURISpec), - EmptyString(), lineNo, EmptyString(), EmptyString()); + EmptyString(), lineNo, columnNo, EmptyString(), EmptyString()); rv = NS_ERROR_SRI_CORRUPT; } } diff --git a/dom/script/nsIScriptElement.h b/dom/script/nsIScriptElement.h index ba3c7dd453..a654291aa0 100644 --- a/dom/script/nsIScriptElement.h +++ b/dom/script/nsIScriptElement.h @@ -30,6 +30,7 @@ public: explicit nsIScriptElement(mozilla::dom::FromParser aFromParser) : mLineNumber(1), + mColumnNumber(1), mAlreadyStarted(false), mMalformed(false), mDoneAddingChildren(aFromParser == mozilla::dom::NOT_FROM_PARSER || @@ -138,6 +139,16 @@ public: return mLineNumber; } + void SetScriptColumnNumber(uint32_t aColumnNumber) + { + mColumnNumber = aColumnNumber; + } + + uint32_t GetScriptColumnNumber() + { + return mColumnNumber; + } + void SetIsMalformed() { mMalformed = true; @@ -281,6 +292,11 @@ protected: */ uint32_t mLineNumber; + /** + * The start column number of the script. + */ + uint32_t mColumnNumber; + /** * The "already started" flag per HTML5. */ diff --git a/dom/security/CSPEvalChecker.cpp b/dom/security/CSPEvalChecker.cpp index 42b5610c7d..51e26a6808 100644 --- a/dom/security/CSPEvalChecker.cpp +++ b/dom/security/CSPEvalChecker.cpp @@ -46,7 +46,7 @@ CheckInternal(nsIContentSecurityPolicy* aCSP, if (reportViolation) { aCSP->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL, aFileNameString, aExpression, aLineNum, - EmptyString(), EmptyString()); + aColumnNum, EmptyString(), EmptyString()); } return NS_OK; diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 5c52d0f201..a52e741e8d 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -40,6 +40,7 @@ #include "nsScriptSecurityManager.h" #include "nsStringStream.h" #include "mozilla/Logging.h" +#include "mozilla/Preferences.h" #include "mozilla/dom/CSPReportBinding.h" #include "mozilla/dom/CSPDictionariesBinding.h" #include "mozilla/net/ReferrerPolicy.h" @@ -271,7 +272,8 @@ nsCSPContext::permitsInternal(CSPDirective aDir, EmptyString(), /* no observer subject */ EmptyString(), /* no source file */ EmptyString(), /* no script sample */ - 0); /* no line number */ + 0, /* no line number */ + 0); /* no column number */ } } } @@ -298,6 +300,14 @@ nsCSPContext::nsCSPContext() , mLoadingPrincipal(nullptr) , mQueueUpMessages(true) { + static bool sInitialized = false; + if (!sInitialized) { + Preferences::AddIntVarCache(&sScriptSampleMaxLength, + "security.csp.reporting.script-sample.max-length", + 40); + sInitialized = true; + } + CSPCONTEXTLOG(("nsCSPContext::nsCSPContext")); } @@ -470,7 +480,8 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, const nsAString& aContent, const nsAString& aViolatedDirective, uint32_t aViolatedPolicyIndex, // TODO, use report only flag for that - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { nsString observerSubject; // if the nonce is non empty, then we report the nonce error, otherwise @@ -500,9 +511,9 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, } nsAutoString codeSample(aContent); - // cap the length of the script sample at 40 chars - if (codeSample.Length() > 40) { - codeSample.Truncate(40); + // cap the length of the script sample + if (codeSample.Length() > ScriptSampleMaxLength()) { + codeSample.Truncate(ScriptSampleMaxLength()); codeSample.AppendLiteral("..."); } AsyncReportViolation(selfISupports, // aBlockedContentSource @@ -512,7 +523,8 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, observerSubject, // aObserverSubject NS_ConvertUTF8toUTF16(sourceFile), // aSourceFile codeSample, // aScriptSample - aLineNumber); // aLineNum + aLineNumber, // aLineNum + aColumnNumber); // aColumnNum } NS_IMETHODIMP @@ -521,6 +533,7 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, bool aParserCreated, const nsAString& aContent, uint32_t aLineNumber, + uint32_t aColumnNumber, bool* outAllowsInline) { *outAllowsInline = true; @@ -565,7 +578,8 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, aContent, violatedDirective, i, - aLineNumber); + aLineNumber, + aColumnNumber); } } return NS_OK; @@ -584,7 +598,8 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, * which is why we must check allows() again here. * * Note: This macro uses some parameters from its caller's context: - * p, mPolicies, this, aSourceFile, aScriptSample, aLineNum, selfISupports + * p, mPolicies, this, aSourceFile, aScriptSample, aLineNum, aColumnNum, + * selfISupports * * @param violationType: the VIOLATION_TYPE_* constant (partial symbol) * such as INLINE_SCRIPT @@ -611,8 +626,8 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, nsIContentPolicy::TYPE_ ## contentPolicyType, \ violatedDirective); \ this->AsyncReportViolation(selfISupports, nullptr, violatedDirective, p, \ - NS_LITERAL_STRING(observerTopic), \ - aSourceFile, aScriptSample, aLineNum); \ + NS_LITERAL_STRING(observerTopic), aSourceFile,\ + aScriptSample, aLineNum, aColumnNum); \ } \ PR_END_MACRO; \ break @@ -644,6 +659,7 @@ nsCSPContext::LogViolationDetails(uint16_t aViolationType, const nsAString& aSourceFile, const nsAString& aScriptSample, int32_t aLineNum, + int32_t aColumnNum, const nsAString& aNonce, const nsAString& aContent) { @@ -844,6 +860,7 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( nsAString& aSourceFile, nsAString& aScriptSample, uint32_t aLineNum, + uint32_t aColumnNum, mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) { NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1); @@ -907,8 +924,19 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( aViolationEventInit.mSourceFile = aSourceFile; } - // sample + // sample, max 40 chars. aViolationEventInit.mSample = aScriptSample; + uint32_t length = aViolationEventInit.mSample.Length(); + if (length > ScriptSampleMaxLength()) { + uint32_t desiredLength = ScriptSampleMaxLength(); + // Don't cut off right before a low surrogate. Just include it. + if (NS_IS_LOW_SURROGATE(aViolationEventInit.mSample[desiredLength])) { + desiredLength++; + } + aViolationEventInit.mSample.Replace(ScriptSampleMaxLength(), + length - desiredLength, + nsContentUtils::GetLocalizedEllipsis()); + } // disposition aViolationEventInit.mDisposition = mPolicies[aViolatedPolicyIndex]->getReportOnlyFlag() @@ -936,8 +964,7 @@ nsCSPContext::GatherSecurityPolicyViolationEventData( aViolationEventInit.mLineNumber = aLineNum; // column-number - // TODO: Set correct column number. - aViolationEventInit.mColumnNumber = 0; + aViolationEventInit.mColumnNumber = aColumnNum; aViolationEventInit.mBubbles = true; aViolationEventInit.mComposed = true; @@ -1012,7 +1039,8 @@ nsCSPContext::SendReports( reportURICstring.get())); logToConsole(u"triedToSendReport", params, ArrayLength(params), aViolationEventInit.mSourceFile, aViolationEventInit.mSample, - aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); + aViolationEventInit.mLineNumber, aViolationEventInit.mColumnNumber, + nsIScriptError::errorFlag); continue; // don't return yet, there may be more URIs } @@ -1054,7 +1082,8 @@ nsCSPContext::SendReports( const char16_t* params[] = { reportURIs[r].get() }; logToConsole(u"reportURInotHttpsOrHttp2", params, ArrayLength(params), aViolationEventInit.mSourceFile, aViolationEventInit.mSample, - aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); + aViolationEventInit.mLineNumber, aViolationEventInit.mColumnNumber, + nsIScriptError::errorFlag); continue; } @@ -1119,7 +1148,8 @@ nsCSPContext::SendReports( CSPCONTEXTLOG(("AsyncOpen failed for report URI %s", params[0])); logToConsole(u"triedToSendReport", params, ArrayLength(params), aViolationEventInit.mSourceFile, aViolationEventInit.mSample, - aViolationEventInit.mLineNumber, 0, nsIScriptError::errorFlag); + aViolationEventInit.mLineNumber, aViolationEventInit.mColumnNumber, + nsIScriptError::errorFlag); } else { CSPCONTEXTLOG(("Sent violation report to URI %s", reportURICstring.get())); } @@ -1162,6 +1192,7 @@ class CSPReportSenderRunnable final : public Runnable const nsAString& aSourceFile, const nsAString& aScriptSample, uint32_t aLineNum, + uint32_t aColumnNum, nsCSPContext* aCSPContext) : mBlockedContentSource(aBlockedContentSource) , mOriginalURI(aOriginalURI) @@ -1171,6 +1202,7 @@ class CSPReportSenderRunnable final : public Runnable , mSourceFile(aSourceFile) , mScriptSample(aScriptSample) , mLineNum(aLineNum) + , mColumnNum(aColumnNum) , mCSPContext(aCSPContext) { NS_ASSERTION(!aViolatedDirective.IsEmpty(), "Can not send reports without a violated directive"); @@ -1208,7 +1240,7 @@ class CSPReportSenderRunnable final : public Runnable blockedURI, blockedDataStr, mOriginalURI, mViolatedDirective, mViolatedPolicyIndex, mSourceFile, mScriptSample, mLineNum, - init); + mColumnNum, init); NS_ENSURE_SUCCESS(rv, rv); // 1) notify observers @@ -1223,15 +1255,22 @@ class CSPReportSenderRunnable final : public Runnable mCSPContext->SendReports(init, mViolatedPolicyIndex); // 3) log to console (one per policy violation) + // if mBlockedContentSource is not a URI, it could be a string + nsCOMPtr blockedString = do_QueryInterface(mBlockedContentSource); if (blockedURI) { blockedURI->GetSpec(blockedDataStr); - bool isData = false; - rv = blockedURI->SchemeIs("data", &isData); - if (NS_SUCCEEDED(rv) && isData) { - blockedDataStr.Truncate(40); - blockedDataStr.AppendASCII("..."); + if (blockedDataStr.Length() > nsCSPContext::ScriptSampleMaxLength()) { + bool isData = false; + rv = blockedURI->SchemeIs("data", &isData); + if (NS_SUCCEEDED(rv) && isData && + blockedDataStr.Length() > nsCSPContext::ScriptSampleMaxLength()) { + blockedDataStr.Truncate(nsCSPContext::ScriptSampleMaxLength()); + blockedDataStr.Append(NS_ConvertUTF16toUTF8(nsContentUtils::GetLocalizedEllipsis())); + } } + } else if (blockedString) { + blockedString->GetData(blockedDataStr); } if (blockedDataStr.Length() > 0) { @@ -1241,7 +1280,7 @@ class CSPReportSenderRunnable final : public Runnable mCSPContext->logToConsole(mReportOnlyFlag ? u"CSPROViolationWithURI" : u"CSPViolationWithURI", params, ArrayLength(params), mSourceFile, mScriptSample, - mLineNum, 0, nsIScriptError::errorFlag); + mLineNum, mColumnNum, nsIScriptError::errorFlag); } // 4) fire violation event @@ -1260,6 +1299,7 @@ class CSPReportSenderRunnable final : public Runnable nsString mSourceFile; nsString mScriptSample; uint32_t mLineNum; + uint32_t mColumnNum; RefPtr mCSPContext; }; @@ -1287,6 +1327,8 @@ class CSPReportSenderRunnable final : public Runnable * a sample of the violating inline script * @param aLineNum * source line number of the violation (if available) + * @param aColumnNum + * source column number of the violation (if available) */ nsresult nsCSPContext::AsyncReportViolation(nsISupports* aBlockedContentSource, @@ -1296,7 +1338,8 @@ nsCSPContext::AsyncReportViolation(nsISupports* aBlockedContentSource, const nsAString& aObserverSubject, const nsAString& aSourceFile, const nsAString& aScriptSample, - uint32_t aLineNum) + uint32_t aLineNum, + uint32_t aColumnNum) { NS_ENSURE_ARG_MAX(aViolatedPolicyIndex, mPolicies.Length() - 1); @@ -1309,6 +1352,7 @@ nsCSPContext::AsyncReportViolation(nsISupports* aBlockedContentSource, aSourceFile, aScriptSample, aLineNum, + aColumnNum, this)); return NS_OK; } diff --git a/dom/security/nsCSPContext.h b/dom/security/nsCSPContext.h index 1d3122af80..272e4c733a 100644 --- a/dom/security/nsCSPContext.h +++ b/dom/security/nsCSPContext.h @@ -85,6 +85,7 @@ class nsCSPContext : public nsIContentSecurityPolicy nsAString& aSourceFile, nsAString& aScriptSample, uint32_t aLineNum, + uint32_t aColumnNum, mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit); nsresult SendReports( @@ -101,7 +102,8 @@ class nsCSPContext : public nsIContentSecurityPolicy const nsAString& aObserverSubject, const nsAString& aSourceFile, const nsAString& aScriptSample, - uint32_t aLineNum); + uint32_t aLineNum, + uint32_t aColumnNum); // Hands off! Don't call this method unless you know what you // are doing. It's only supposed to be called from within @@ -110,10 +112,14 @@ class nsCSPContext : public nsIContentSecurityPolicy mLoadingPrincipal = nullptr; } - nsWeakPtr GetLoadingContext(){ + nsWeakPtr GetLoadingContext() { return mLoadingContext; } + static uint32_t ScriptSampleMaxLength() { + return std::max(sScriptSampleMaxLength, 0); + } + private: bool permitsInternal(CSPDirective aDir, nsIURI* aContentLocation, @@ -132,7 +138,10 @@ class nsCSPContext : public nsIContentSecurityPolicy const nsAString& aContent, const nsAString& aViolatedDirective, uint32_t aViolatedPolicyIndex, - uint32_t aLineNumber); + uint32_t aLineNumber, + uint32_t aColumnNumber); + + static int32_t sScriptSampleMaxLength; nsString mReferrer; uint64_t mInnerWindowID; // used for web console logging diff --git a/dom/smil/nsSMILCSSValueType.cpp b/dom/smil/nsSMILCSSValueType.cpp index 71ef27cd63..93e05aca07 100644 --- a/dom/smil/nsSMILCSSValueType.cpp +++ b/dom/smil/nsSMILCSSValueType.cpp @@ -405,7 +405,7 @@ nsSMILCSSValueType::ValueFromString(nsCSSPropertyID aPropID, if (doc && !nsStyleUtil::CSPAllowsInlineStyle(nullptr, doc->NodePrincipal(), doc->GetDocumentURI(), - 0, aString, nullptr)) { + 0, 0, aString, nullptr)) { return; } diff --git a/dom/webidl/CSPReport.webidl b/dom/webidl/CSPReport.webidl index 390898c83d..301ca2885c 100644 --- a/dom/webidl/CSPReport.webidl +++ b/dom/webidl/CSPReport.webidl @@ -16,6 +16,7 @@ dictionary CSPReportProperties { DOMString source-file; DOMString script-sample; long line-number; + long column-number; }; dictionary CSPReport { diff --git a/dom/workers/RuntimeService.cpp b/dom/workers/RuntimeService.cpp index 199f6ea56a..0309a67375 100644 --- a/dom/workers/RuntimeService.cpp +++ b/dom/workers/RuntimeService.cpp @@ -547,14 +547,21 @@ class LogViolationDetailsRunnable final : public WorkerMainThreadRunnable { nsString mFileName; uint32_t mLineNum; + uint32_t mColumnNum; + nsString mScriptSample; public: LogViolationDetailsRunnable(WorkerPrivate* aWorker, const nsString& aFileName, - uint32_t aLineNum) + uint32_t aLineNum, + uint32_t aColumnNum, + const nsAString& aScriptSample) : WorkerMainThreadRunnable(aWorker, NS_LITERAL_CSTRING("RuntimeService :: LogViolationDetails")) - , mFileName(aFileName), mLineNum(aLineNum) + , mFileName(aFileName) + , mLineNum(aLineNum) + , mColumnNum(aColumnNum) + , mScriptSample(aScriptSample) { MOZ_ASSERT(aWorker); } @@ -566,24 +573,38 @@ private: }; bool -ContentSecurityPolicyAllows(JSContext* aCx) +ContentSecurityPolicyAllows(JSContext* aCx, JS::HandleValue aValue) { WorkerPrivate* worker = GetWorkerPrivateFromContext(aCx); worker->AssertIsOnWorkerThread(); if (worker->GetReportCSPViolations()) { + JS::Rooted jsString(aCx, JS::ToString(aCx, aValue)); + if (NS_WARN_IF(!jsString)) { + JS_ClearPendingException(aCx); + return false; + } + + nsAutoJSString scriptSample; + if (NS_WARN_IF(!scriptSample.init(aCx, jsString))) { + JS_ClearPendingException(aCx); + return false; + } + nsString fileName; uint32_t lineNum = 0; + uint32_t columnNum = 0; JS::AutoFilename file; - if (JS::DescribeScriptedCaller(aCx, &file, &lineNum) && file.get()) { + if (JS::DescribeScriptedCaller(aCx, &file, &lineNum, &columnNum) && file.get()) { fileName = NS_ConvertUTF8toUTF16(file.get()); } else { MOZ_ASSERT(!JS_IsExceptionPending(aCx)); } RefPtr runnable = - new LogViolationDetailsRunnable(worker, fileName, lineNum); + new LogViolationDetailsRunnable(worker, fileName, lineNum, columnNum, + scriptSample); ErrorResult rv; runnable->Dispatch(Killing, rv); @@ -2698,11 +2719,9 @@ LogViolationDetailsRunnable::MainThreadRun() nsIContentSecurityPolicy* csp = mWorkerPrivate->GetCSP(); if (csp) { - NS_NAMED_LITERAL_STRING(scriptSample, - "Call to eval() or related function blocked by CSP."); if (mWorkerPrivate->GetReportCSPViolations()) { csp->LogViolationDetails(nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL, - mFileName, scriptSample, mLineNum, + mFileName, mScriptSample, mLineNum, mColumnNum, EmptyString(), EmptyString()); } } diff --git a/dom/workers/ScriptLoader.cpp b/dom/workers/ScriptLoader.cpp index 646182bfd3..561759cc4c 100644 --- a/dom/workers/ScriptLoader.cpp +++ b/dom/workers/ScriptLoader.cpp @@ -1167,7 +1167,7 @@ private: if (wcsp) { wcsp->LogViolationDetails( nsIContentSecurityPolicy::VIOLATION_TYPE_REQUIRE_SRI_FOR_SCRIPT, - aLoadInfo.mURL, EmptyString(), 0, EmptyString(), EmptyString()); + aLoadInfo.mURL, EmptyString(), 0, 0, EmptyString(), EmptyString()); } return NS_ERROR_SRI_CORRUPT; } diff --git a/dom/xbl/nsXBLContentSink.cpp b/dom/xbl/nsXBLContentSink.cpp index 0f54d62b49..3232f2a0fb 100644 --- a/dom/xbl/nsXBLContentSink.cpp +++ b/dom/xbl/nsXBLContentSink.cpp @@ -248,10 +248,11 @@ NS_IMETHODIMP nsXBLContentSink::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { nsresult rv = nsXMLContentSink::HandleStartElement(aName, aAtts, aAttsCount, - aLineNumber); + aLineNumber, aColumnNumber); if (NS_FAILED(rv)) return rv; @@ -850,7 +851,8 @@ nsXBLContentSink::ConstructParameter(const char16_t **aAtts) nsresult nsXBLContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, FromParser aFromParser) { @@ -858,7 +860,7 @@ nsXBLContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, if (!aNodeInfo->NamespaceEquals(kNameSpaceID_XUL)) { #endif return nsXMLContentSink::CreateElement(aAtts, aAttsCount, aNodeInfo, - aLineNumber, aResult, + aLineNumber, aColumnNumber, aResult, aAppendContent, aFromParser); #ifdef MOZ_XUL } diff --git a/dom/xbl/nsXBLContentSink.h b/dom/xbl/nsXBLContentSink.h index 93c1454a40..a09ee78b07 100644 --- a/dom/xbl/nsXBLContentSink.h +++ b/dom/xbl/nsXBLContentSink.h @@ -69,7 +69,8 @@ public: NS_IMETHOD HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) override; + uint32_t aLineNumber, + uint32_t aColumnNumber) override; NS_IMETHOD HandleEndElement(const char16_t *aName) override; @@ -89,7 +90,8 @@ protected: bool NotifyForDocElement() override { return false; } nsresult CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, mozilla::dom::FromParser aFromParser) override; diff --git a/dom/xml/nsXMLContentSink.cpp b/dom/xml/nsXMLContentSink.cpp index 1e6f35eb89..7db1ea4a6a 100644 --- a/dom/xml/nsXMLContentSink.cpp +++ b/dom/xml/nsXMLContentSink.cpp @@ -446,7 +446,8 @@ nsXMLContentSink::SetParser(nsParserBase* aParser) nsresult nsXMLContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, FromParser aFromParser) { @@ -466,6 +467,7 @@ nsXMLContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, ) { nsCOMPtr sele = do_QueryInterface(content); sele->SetScriptLineNumber(aLineNumber); + sele->SetScriptColumnNumber(aColumnNumber); sele->SetCreatorParser(GetParser()); } @@ -500,6 +502,7 @@ nsXMLContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, } if (!aNodeInfo->Equals(nsGkAtoms::link, kNameSpaceID_XHTML)) { ssle->SetLineNumber(aFromParser ? aLineNumber : 0); + ssle->SetColumnNumber(aFromParser ? aColumnNumber : 0); } } } @@ -918,10 +921,11 @@ NS_IMETHODIMP nsXMLContentSink::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { return HandleStartElement(aName, aAtts, aAttsCount, aLineNumber, - true); + aColumnNumber, true); } nsresult @@ -929,6 +933,7 @@ nsXMLContentSink::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, uint32_t aLineNumber, + uint32_t aColumnNumber, bool aInterruptable) { NS_PRECONDITION(aAttsCount % 2 == 0, "incorrect aAttsCount"); @@ -963,7 +968,7 @@ nsXMLContentSink::HandleStartElement(const char16_t *aName, nsIDOMNode::ELEMENT_NODE); result = CreateElement(aAtts, aAttsCount, nodeInfo, aLineNumber, - getter_AddRefs(content), &appendContent, + aColumnNumber, getter_AddRefs(content), &appendContent, FROM_PARSER_NETWORK); NS_ENSURE_SUCCESS(result, result); diff --git a/dom/xml/nsXMLContentSink.h b/dom/xml/nsXMLContentSink.h index f08f99a3a0..ea190954a2 100644 --- a/dom/xml/nsXMLContentSink.h +++ b/dom/xml/nsXMLContentSink.h @@ -114,7 +114,8 @@ protected: nsIContent *aContent); virtual bool NotifyForDocElement() { return true; } virtual nsresult CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, mozilla::dom::FromParser aFromParser); @@ -161,7 +162,7 @@ protected: nsresult HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, uint32_t aLineNumber, - bool aInterruptable); + uint32_t aColumnNumber, bool aInterruptable); nsresult HandleEndElement(const char16_t *aName, bool aInterruptable); nsresult HandleCharacterData(const char16_t *aData, uint32_t aLength, bool aInterruptable); diff --git a/dom/xml/nsXMLFragmentContentSink.cpp b/dom/xml/nsXMLFragmentContentSink.cpp index 04d688c81e..a4bb406351 100644 --- a/dom/xml/nsXMLFragmentContentSink.cpp +++ b/dom/xml/nsXMLFragmentContentSink.cpp @@ -83,7 +83,8 @@ protected: nsIAtom* aTagName, nsIContent* aContent) override; virtual nsresult CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, mozilla::dom::FromParser aFromParser) override; virtual nsresult CloseElement(nsIContent* aContent) override; @@ -199,7 +200,8 @@ nsXMLFragmentContentSink::SetDocElement(int32_t aNameSpaceID, nsresult nsXMLFragmentContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCount, - mozilla::dom::NodeInfo* aNodeInfo, uint32_t aLineNumber, + mozilla::dom::NodeInfo* aNodeInfo, + uint32_t aLineNumber, uint32_t aColumnNumber, nsIContent** aResult, bool* aAppendContent, FromParser /*aFromParser*/) { @@ -207,6 +209,7 @@ nsXMLFragmentContentSink::CreateElement(const char16_t** aAtts, uint32_t aAttsCo // fancy CloseElement stuff. nsresult rv = nsXMLContentSink::CreateElement(aAtts, aAttsCount, aNodeInfo, aLineNumber, + aColumnNumber, aResult, aAppendContent, NOT_FROM_PARSER); diff --git a/dom/xslt/xslt/txMozillaStylesheetCompiler.cpp b/dom/xslt/xslt/txMozillaStylesheetCompiler.cpp index c9bcc31ff0..6137e87c11 100644 --- a/dom/xslt/xslt/txMozillaStylesheetCompiler.cpp +++ b/dom/xslt/xslt/txMozillaStylesheetCompiler.cpp @@ -122,7 +122,8 @@ NS_IMETHODIMP txStylesheetSink::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { NS_PRECONDITION(aAttsCount % 2 == 0, "incorrect aAttsCount"); diff --git a/dom/xul/nsXULContentSink.cpp b/dom/xul/nsXULContentSink.cpp index c631e0424d..28e269d59f 100644 --- a/dom/xul/nsXULContentSink.cpp +++ b/dom/xul/nsXULContentSink.cpp @@ -437,7 +437,8 @@ NS_IMETHODIMP XULContentSinkImpl::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { // XXX Hopefully the parser will flag this before we get here. If // we're in the epilog, there should be no new elements @@ -694,7 +695,7 @@ XULContentSinkImpl::ReportError(const char16_t* aErrorText, parsererror.Append((char16_t)0xFFFF); parsererror.AppendLiteral("parsererror"); - rv = HandleStartElement(parsererror.get(), noAtts, 0, 0); + rv = HandleStartElement(parsererror.get(), noAtts, 0, 0, 0); NS_ENSURE_SUCCESS(rv,rv); rv = HandleCharacterData(aErrorText, NS_strlen(aErrorText)); @@ -704,7 +705,7 @@ XULContentSinkImpl::ReportError(const char16_t* aErrorText, sourcetext.Append((char16_t)0xFFFF); sourcetext.AppendLiteral("sourcetext"); - rv = HandleStartElement(sourcetext.get(), noAtts, 0, 0); + rv = HandleStartElement(sourcetext.get(), noAtts, 0, 0, 0); NS_ENSURE_SUCCESS(rv,rv); rv = HandleCharacterData(aSourceText, NS_strlen(aSourceText)); diff --git a/js/public/Principals.h b/js/public/Principals.h index 4f2670b653..ec7083d768 100644 --- a/js/public/Principals.h +++ b/js/public/Principals.h @@ -64,10 +64,10 @@ typedef bool /* * Used to check if a CSP instance wants to disable eval() and friends. - * See js_CheckCSPPermitsJSAction() in jsobj. + * See GlobalObject::isRuntimeCodeGenEnabled() in vm/GlobalObject.cpp. */ typedef bool -(* JSCSPEvalChecker)(JSContext* cx); +(* JSCSPEvalChecker)(JSContext* cx, JS::HandleValue aValue); struct JSSecurityCallbacks { JSCSPEvalChecker contentSecurityPolicyAllows; diff --git a/js/src/builtin/Eval.cpp b/js/src/builtin/Eval.cpp index 53fa789313..b60330b516 100644 --- a/js/src/builtin/Eval.cpp +++ b/js/src/builtin/Eval.cpp @@ -227,7 +227,7 @@ EvalKernel(JSContext* cx, HandleValue v, EvalType evalType, AbstractFramePtr cal AssertInnerizedEnvironmentChain(cx, *env); Rooted envGlobal(cx, &env->global()); - if (!GlobalObject::isRuntimeCodeGenEnabled(cx, envGlobal)) { + if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, envGlobal)) { JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CSP_BLOCKED_EVAL); return false; } @@ -330,7 +330,8 @@ js::DirectEvalStringFromIon(JSContext* cx, AssertInnerizedEnvironmentChain(cx, *env); Rooted envGlobal(cx, &env->global()); - if (!GlobalObject::isRuntimeCodeGenEnabled(cx, envGlobal)) { + RootedValue v(cx, StringValue(str)); + if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, envGlobal)) { JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CSP_BLOCKED_EVAL); return false; } diff --git a/js/src/jsfun.cpp b/js/src/jsfun.cpp index ec13ce3e9a..ad2e1fcd95 100644 --- a/js/src/jsfun.cpp +++ b/js/src/jsfun.cpp @@ -1625,13 +1625,6 @@ static bool FunctionConstructor(JSContext* cx, const CallArgs& args, GeneratorKind generatorKind, FunctionAsyncKind asyncKind) { - // Block this call if security callbacks forbid it. - Rooted global(cx, &args.callee().global()); - if (!GlobalObject::isRuntimeCodeGenEnabled(cx, global)) { - JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CSP_BLOCKED_FUNCTION); - return false; - } - bool isStarGenerator = generatorKind == StarGenerator; bool isAsync = asyncKind == AsyncFunction; MOZ_ASSERT(generatorKind != LegacyGenerator); @@ -1733,6 +1726,14 @@ FunctionConstructor(JSContext* cx, const CallArgs& args, GeneratorKind generator if (!functionText) return false; + // Block this call if security callbacks forbid it. + Rooted global(cx, &args.callee().global()); + RootedValue v(cx, StringValue(functionText)); + if (!GlobalObject::isRuntimeCodeGenEnabled(cx, v, global)) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_CSP_BLOCKED_FUNCTION); + return false; + } + /* * NB: (new Function) is not lexically closed by its caller, it's just an * anonymous function in the top-level scope that its constructor inhabits. diff --git a/js/src/vm/GlobalObject.cpp b/js/src/vm/GlobalObject.cpp index 1321be9e9b..8d73fa15cc 100644 --- a/js/src/vm/GlobalObject.cpp +++ b/js/src/vm/GlobalObject.cpp @@ -543,17 +543,23 @@ GlobalObject::initSelfHostingBuiltins(JSContext* cx, Handle globa } /* static */ bool -GlobalObject::isRuntimeCodeGenEnabled(JSContext* cx, Handle global) +GlobalObject::isRuntimeCodeGenEnabled(JSContext* cx, HandleValue code, + Handle global) { HeapSlot& v = global->getSlotRef(RUNTIME_CODEGEN_ENABLED); if (v.isUndefined()) { /* * If there are callbacks, make sure that the CSP callback is installed - * and that it permits runtime code generation, then cache the result. + * and that it permits runtime code generation. */ JSCSPEvalChecker allows = cx->runtime()->securityCallbacks->contentSecurityPolicyAllows; - Value boolValue = BooleanValue(!allows || allows(cx)); - v.set(global, HeapSlot::Slot, RUNTIME_CODEGEN_ENABLED, boolValue); + if (allows) + return allows(cx, code); + + // Let's cache the result only if the contentSecurityPolicyAllows callback is not set. In + // this way, contentSecurityPolicyAllows callback is executed each time, with the current + // HandleValue code. + v.set(global, HeapSlot::Slot, RUNTIME_CODEGEN_ENABLED, JS::TrueValue()); } return !v.isFalse(); } diff --git a/js/src/vm/GlobalObject.h b/js/src/vm/GlobalObject.h index 355d055fb6..9aa47638d0 100644 --- a/js/src/vm/GlobalObject.h +++ b/js/src/vm/GlobalObject.h @@ -803,7 +803,8 @@ class GlobalObject : public NativeObject template inline Value createArrayFromBuffer() const; - static bool isRuntimeCodeGenEnabled(JSContext* cx, Handle global); + static bool isRuntimeCodeGenEnabled(JSContext* cx, HandleValue code, + Handle global); // Warn about use of the deprecated watch/unwatch functions in the global // in which |obj| was created, if no prior warning was given. diff --git a/layout/style/Loader.cpp b/layout/style/Loader.cpp index 48ca1739dc..ff4ff80eba 100644 --- a/layout/style/Loader.cpp +++ b/layout/style/Loader.cpp @@ -955,7 +955,7 @@ SheetLoadData::OnStreamComplete(nsIUnicharStreamLoader* aLoader, csp->LogViolationDetails( nsIContentSecurityPolicy::VIOLATION_TYPE_REQUIRE_SRI_FOR_STYLE, NS_ConvertUTF8toUTF16(spec), EmptyString(), - 0, EmptyString(), EmptyString()); + 0, 0, EmptyString(), EmptyString()); return NS_OK; } } else { diff --git a/layout/style/nsStyleUtil.cpp b/layout/style/nsStyleUtil.cpp index 9c3c0f449a..274f5140f2 100644 --- a/layout/style/nsStyleUtil.cpp +++ b/layout/style/nsStyleUtil.cpp @@ -739,6 +739,7 @@ nsStyleUtil::CSPAllowsInlineStyle(nsIContent* aContent, nsIPrincipal* aPrincipal, nsIURI* aSourceURI, uint32_t aLineNumber, + uint32_t aColumnNumber, const nsSubstring& aStyleText, nsresult* aRv) { @@ -776,7 +777,7 @@ nsStyleUtil::CSPAllowsInlineStyle(nsIContent* aContent, rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_STYLESHEET, nonce, false, // aParserCreated only applies to scripts - aStyleText, aLineNumber, + aStyleText, aLineNumber, aColumnNumber, &allowInlineStyle); NS_ENSURE_SUCCESS(rv, false); diff --git a/layout/style/nsStyleUtil.h b/layout/style/nsStyleUtil.h index e5b7a055ff..95cc2c5ac6 100644 --- a/layout/style/nsStyleUtil.h +++ b/layout/style/nsStyleUtil.h @@ -164,6 +164,9 @@ public: * @param aLineNumber * Line number of inline style element in the containing document (for * reporting violations) + * @param aColumnNumber + * Column number of inline style element in the containing document (for + * reporting violations) * @param aStyleText * Contents of the inline style element (for reporting violations) * @param aRv @@ -175,6 +178,7 @@ public: nsIPrincipal* aPrincipal, nsIURI* aSourceURI, uint32_t aLineNumber, + uint32_t aColumnNumber, const nsSubstring& aStyleText, nsresult* aRv); diff --git a/parser/htmlparser/nsExpatDriver.cpp b/parser/htmlparser/nsExpatDriver.cpp index e35a1da256..743a6547d2 100644 --- a/parser/htmlparser/nsExpatDriver.cpp +++ b/parser/htmlparser/nsExpatDriver.cpp @@ -394,7 +394,8 @@ nsExpatDriver::HandleStartElement(const char16_t *aValue, nsresult rv = mSink-> HandleStartElement(aValue, aAtts, attrArrayLength, - XML_GetCurrentLineNumber(mExpatParser)); + XML_GetCurrentLineNumber(mExpatParser), + XML_GetCurrentColumnNumber(mExpatParser)); MaybeStopParser(rv); } } diff --git a/parser/htmlparser/nsIExpatSink.idl b/parser/htmlparser/nsIExpatSink.idl index df0b2d869f..d8217b60c8 100644 --- a/parser/htmlparser/nsIExpatSink.idl +++ b/parser/htmlparser/nsIExpatSink.idl @@ -28,11 +28,13 @@ interface nsIExpatSink : nsISupports * present in aAtts. * @param aAttsCount the number of elements in aAtts. * @param aLineNumber the line number of the start tag in the data stream. + * @param aColumnNumber the column number of the start tag in the data stream. */ void HandleStartElement(in wstring aName, [array, size_is(aAttsCount)] in wstring aAtts, in unsigned long aAttsCount, - in unsigned long aLineNumber); + in unsigned long aLineNumber, + in unsigned long aColumnNumber); /** * Called to handle the closing tag of an element. diff --git a/parser/xml/nsSAXXMLReader.cpp b/parser/xml/nsSAXXMLReader.cpp index a84e0d63ba..2d0d23569d 100644 --- a/parser/xml/nsSAXXMLReader.cpp +++ b/parser/xml/nsSAXXMLReader.cpp @@ -82,7 +82,8 @@ NS_IMETHODIMP nsSAXXMLReader::HandleStartElement(const char16_t *aName, const char16_t **aAtts, uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aLineNumber, + uint32_t aColumnNumber) { if (!mContentHandler) return NS_OK; diff --git a/rdf/base/nsRDFContentSink.cpp b/rdf/base/nsRDFContentSink.cpp index ae05a9381b..ec9eb2fc8d 100644 --- a/rdf/base/nsRDFContentSink.cpp +++ b/rdf/base/nsRDFContentSink.cpp @@ -388,8 +388,9 @@ RDFContentSinkImpl::QueryInterface(REFNSIID iid, void** result) NS_IMETHODIMP RDFContentSinkImpl::HandleStartElement(const char16_t *aName, const char16_t **aAtts, - uint32_t aAttsCount, - uint32_t aLineNumber) + uint32_t aAttsCount, + uint32_t aLineNumber, + uint32_t aColumnNumber) { FlushText(); From e21d466d13dca371148fddbbd81eae837fca1c83 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 8 Jan 2024 08:49:05 -0600 Subject: [PATCH 21/25] Issue #2402 - Print Related JS-Line on CSP Violation (if any). https://bugzilla.mozilla.org/show_bug.cgi?id=1100630 --- dom/security/nsCSPContext.cpp | 47 +++++++++++++++++++++++++++-------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index a52e741e8d..9a71f9b2eb 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -264,16 +264,25 @@ nsCSPContext::permitsInternal(CSPDirective aDir, // decision may be wrong due to the inability to get the nonce, and will // incorrectly fail the unit tests. if (!aIsPreload && aSendViolationReports) { + uint32_t lineNumber = 0; + uint32_t columnNumber = 0; + nsAutoCString spec; + JSContext* cx = nsContentUtils::GetCurrentJSContext(); + if (cx) { + nsJSUtils::GetCallingLocation(cx, spec, &lineNumber, &columnNumber); + // If GetCallingLocation fails linenumber & columnNumber are set to 0 + // anyway so we can skip checking if that is the case. + } this->AsyncReportViolation((aSendContentLocationInViolationReports ? aContentLocation : nullptr), - aOriginalURI, /* in case of redirect originalURI is not null */ - violatedDirective, - p, /* policy index */ - EmptyString(), /* no observer subject */ - EmptyString(), /* no source file */ - EmptyString(), /* no script sample */ - 0, /* no line number */ - 0); /* no column number */ + aOriginalURI, /* in case of redirect originalURI is not null */ + violatedDirective, + p, /* policy index */ + EmptyString(), /* no observer subject */ + NS_ConvertUTF8toUTF16(spec), /* source file. */ + EmptyString(), /* no script sample */ + lineNumber, /* line number */ + columnNumber); /* column number */ } } } @@ -294,6 +303,8 @@ NS_IMPL_ISUPPORTS_CI(nsCSPContext, nsIContentSecurityPolicy, nsISerializable) +int32_t nsCSPContext::sScriptSampleMaxLength; + nsCSPContext::nsCSPContext() : mInnerWindowID(0) , mLoadingContext(nullptr) @@ -516,6 +527,22 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, codeSample.Truncate(ScriptSampleMaxLength()); codeSample.AppendLiteral("..."); } + + uint32_t lineNumber = aLineNumber; + uint32_t columnNumber = aColumnNumber; + + JSContext* cx = nsContentUtils::GetCurrentJSContext(); + if (cx) { + if (!nsJSUtils::GetCallingLocation(cx, sourceFile, &lineNumber, + &columnNumber)) { + // Get Calling Location resets line/col to 0 + // so we reset those to the intial arguments + // in case it failed + lineNumber = aLineNumber; + columnNumber = aColumnNumber; + } + } + AsyncReportViolation(selfISupports, // aBlockedContentSource mSelfURI, // aOriginalURI aViolatedDirective, // aViolatedDirective @@ -523,8 +550,8 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType, observerSubject, // aObserverSubject NS_ConvertUTF8toUTF16(sourceFile), // aSourceFile codeSample, // aScriptSample - aLineNumber, // aLineNum - aColumnNumber); // aColumnNum + lineNumber, // aLineNum + columnNumber); // aColumnNum } NS_IMETHODIMP From b842a1937aa30ea7c5514212517c2a3801520e88 Mon Sep 17 00:00:00 2001 From: Brian Smith Date: Mon, 8 Jan 2024 21:14:05 -0600 Subject: [PATCH 22/25] Issue #2402 - Fill in column in CSP Report. --- dom/security/nsCSPContext.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index 9a71f9b2eb..9eafd9498b 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -1041,6 +1041,11 @@ nsCSPContext::SendReports( report.mCsp_report.mLine_number.Value() = aViolationEventInit.mLineNumber; } + if (aViolationEventInit.mColumnNumber != 0) { + report.mCsp_report.mColumn_number.Construct(); + report.mCsp_report.mColumn_number.Value() = aViolationEventInit.mColumnNumber; + } + nsString csp_report; if (!report.ToJSON(csp_report)) { return NS_ERROR_FAILURE; From a73c8e93d6f6bbb0abe317507da2f761114ba693 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 9 Jan 2024 11:30:23 +0100 Subject: [PATCH 23/25] Issue #2401 - Part 1: Templatize GetCommonAncestor. --- dom/base/nsContentUtils.cpp | 30 ++++++++++++++++++++---------- dom/base/nsContentUtils.h | 7 +++---- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index db8ea5fed7..adaf35920e 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -2395,34 +2395,35 @@ nsContentUtils::GetCommonAncestor(nsIDOMNode *aNode, return CallQueryInterface(common, aCommonAncestor); } -// static -nsINode* -nsContentUtils::GetCommonAncestor(nsINode* aNode1, - nsINode* aNode2) +template +static Node* +GetCommonAncestorInternal(Node* aNode1, + Node* aNode2, + GetParentFunc aGetParentFunc) { if (aNode1 == aNode2) { return aNode1; } // Build the chain of parents - AutoTArray parents1, parents2; + AutoTArray parents1, parents2; do { parents1.AppendElement(aNode1); - aNode1 = aNode1->GetParentNode(); + aNode1 = aGetParentFunc(aNode1); } while (aNode1); do { parents2.AppendElement(aNode2); - aNode2 = aNode2->GetParentNode(); + aNode2 = aGetParentFunc(aNode2); } while (aNode2); // Find where the parent chain differs uint32_t pos1 = parents1.Length(); uint32_t pos2 = parents2.Length(); - nsINode* parent = nullptr; + Node* parent = nullptr; uint32_t len; for (len = std::min(pos1, pos2); len > 0; --len) { - nsINode* child1 = parents1.ElementAt(--pos1); - nsINode* child2 = parents2.ElementAt(--pos2); + Node* child1 = parents1.ElementAt(--pos1); + Node* child2 = parents2.ElementAt(--pos2); if (child1 != child2) { break; } @@ -2432,6 +2433,15 @@ nsContentUtils::GetCommonAncestor(nsINode* aNode1, return parent; } +/* static */ +nsINode* +nsContentUtils::GetCommonAncestor(nsINode* aNode1, nsINode* aNode2) +{ + return GetCommonAncestorInternal(aNode1, aNode2, [](nsINode* aNode) { + return aNode->GetParentNode(); + }); +} + // static nsINode* nsContentUtils::GetCommonAncestorUnderInteractiveContent(nsINode* aNode1, diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index 74239c8036..c6e8070576 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -333,11 +333,10 @@ public: nsIDOMNode** aCommonAncestor); /** - * Returns the common ancestor, if any, for two nodes. Returns null if the - * nodes are disconnected. + * Returns the common ancestor, if any, for two nodes. + * Returns null if the nodes are disconnected. */ - static nsINode* GetCommonAncestor(nsINode* aNode1, - nsINode* aNode2); + static nsINode* GetCommonAncestor(nsINode* aNode1, nsINode* aNode2); /** * Returns the common ancestor under interactive content, if any. From d3755784153a5251764194b3dec76d611d825a83 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 9 Jan 2024 11:40:33 +0100 Subject: [PATCH 24/25] Issue #2401 - Part 2: Add GetCommonFlattenedTreeAncestor. Passing GetFlattenedTreeParent() as the "get parent" function. This way we can reduce the complexity in our EventStateManager by simply calling out to the templatized function from Part 1. --- dom/base/nsContentUtils.cpp | 10 +++++++ dom/base/nsContentUtils.h | 7 +++++ dom/events/EventStateManager.cpp | 45 ++++---------------------------- 3 files changed, 22 insertions(+), 40 deletions(-) diff --git a/dom/base/nsContentUtils.cpp b/dom/base/nsContentUtils.cpp index adaf35920e..6d092a53d0 100644 --- a/dom/base/nsContentUtils.cpp +++ b/dom/base/nsContentUtils.cpp @@ -2442,6 +2442,16 @@ nsContentUtils::GetCommonAncestor(nsINode* aNode1, nsINode* aNode2) }); } +/* static */ +nsIContent* +nsContentUtils::GetCommonFlattenedTreeAncestor(nsIContent* aContent1, + nsIContent* aContent2) +{ + return GetCommonAncestorInternal(aContent1, aContent2, [](nsIContent* aContent) { + return aContent->GetFlattenedTreeParent(); + }); +} + // static nsINode* nsContentUtils::GetCommonAncestorUnderInteractiveContent(nsINode* aNode1, diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h index c6e8070576..9c22fa491e 100644 --- a/dom/base/nsContentUtils.h +++ b/dom/base/nsContentUtils.h @@ -338,6 +338,13 @@ public: */ static nsINode* GetCommonAncestor(nsINode* aNode1, nsINode* aNode2); + /** + * Returns the common flattened tree ancestor, if any, for two given content + * nodes. + */ + static nsIContent* GetCommonFlattenedTreeAncestor(nsIContent* aContent1, + nsIContent* aContent2); + /** * Returns the common ancestor under interactive content, if any. * If neither one has interactive content as ancestor, common ancestor will be diff --git a/dom/events/EventStateManager.cpp b/dom/events/EventStateManager.cpp index dd753a97f0..371f66af42 100644 --- a/dom/events/EventStateManager.cpp +++ b/dom/events/EventStateManager.cpp @@ -4849,48 +4849,13 @@ GetLabelTarget(nsIContent* aPossibleLabel) return label->GetLabeledElement(); } -static nsIContent* FindCommonAncestor(nsIContent *aNode1, nsIContent *aNode2) +static nsIContent* +FindCommonAncestor(nsIContent *aNode1, nsIContent *aNode2) { - // Find closest common ancestor - if (aNode1 && aNode2) { - // Find the nearest common ancestor by counting the distance to the - // root and then walking up again, in pairs. - int32_t offset = 0; - nsIContent *anc1 = aNode1; - for (;;) { - ++offset; - nsIContent* parent = anc1->GetFlattenedTreeParent(); - if (!parent) - break; - anc1 = parent; - } - nsIContent *anc2 = aNode2; - for (;;) { - --offset; - nsIContent* parent = anc2->GetFlattenedTreeParent(); - if (!parent) - break; - anc2 = parent; - } - if (anc1 == anc2) { - anc1 = aNode1; - anc2 = aNode2; - while (offset > 0) { - anc1 = anc1->GetFlattenedTreeParent(); - --offset; - } - while (offset < 0) { - anc2 = anc2->GetFlattenedTreeParent(); - ++offset; - } - while (anc1 != anc2) { - anc1 = anc1->GetFlattenedTreeParent(); - anc2 = anc2->GetFlattenedTreeParent(); - } - return anc1; - } + if (!aNode1 || !aNode2) { + return nullptr; } - return nullptr; + return nsContentUtils::GetCommonFlattenedTreeAncestor(aNode1, aNode2); } /* static */ From bd723aac2859301a37c128eb9e644cc4478b536e Mon Sep 17 00:00:00 2001 From: Moonchild Date: Tue, 9 Jan 2024 18:23:47 +0100 Subject: [PATCH 25/25] Issue #2401 - Part 3: Don't toggle focus-within state past the nearest common flattened tree ancestor. Resolves #2401 --- dom/base/nsFocusManager.cpp | 39 +++++++++++++++++++++++++++++-------- dom/base/nsFocusManager.h | 1 + 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index a0e162b0a1..fa321ac300 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -538,10 +538,8 @@ nsFocusManager::MoveFocus(mozIDOMWindowProxy* aWindow, nsIDOMElement* aStartElem NS_ENSURE_TRUE(startContent, NS_ERROR_INVALID_ARG); window = GetCurrentWindow(startContent); - } - else { + } else { window = aWindow ? nsPIDOMWindowOuter::From(aWindow) : mFocusedWindow.get(); - NS_ENSURE_TRUE(window, NS_ERROR_FAILURE); } NS_ENSURE_TRUE(window, NS_ERROR_FAILURE); @@ -867,7 +865,7 @@ nsFocusManager::ContentRemoved(nsIDocument* aDocument, nsIContent* aContent) } } - NotifyFocusStateChange(content, shouldShowFocusRing, false); + NotifyFocusStateChange(content, nullptr, shouldShowFocusRing, false); } return NS_OK; @@ -973,6 +971,7 @@ nsFocusManager::WindowHidden(mozIDOMWindowProxy* aWindow) if (oldFocusedContent && oldFocusedContent->IsInComposedDoc()) { NotifyFocusStateChange(oldFocusedContent, + nullptr, mFocusedWindow->ShouldShowFocusRing(), false); window->UpdateCommands(NS_LITERAL_STRING("focus"), nullptr, 0); @@ -1091,12 +1090,21 @@ nsFocusManager::ParentActivated(mozIDOMWindowProxy* aWindow, bool aActive) /* static */ void nsFocusManager::NotifyFocusStateChange(nsIContent* aContent, + nsIContent* aContentToFocus, bool aWindowShouldShowFocusRing, bool aGettingFocus) { + MOZ_ASSERT_IF(aContentToFocus, !aGettingFocus); if (!aContent->IsElement()) { return; } + + nsIContent* commonAncestor = nullptr; + if (aContentToFocus && aContentToFocus->IsElement()) { + commonAncestor = + nsContentUtils::GetCommonFlattenedTreeAncestor(aContent, aContentToFocus); + } + EventStates eventState = NS_EVENT_STATE_FOCUS; if (aWindowShouldShowFocusRing) { eventState |= NS_EVENT_STATE_FOCUSRING; @@ -1108,9 +1116,18 @@ nsFocusManager::NotifyFocusStateChange(nsIContent* aContent, aContent->AsElement()->RemoveStates(eventState); } - for (Element* element = aContent->AsElement(); element; - element = element->GetParentElementCrossingShadowRoot()) { + for (nsIContent* content = aContent; + content && content != commonAncestor; + content = content->GetFlattenedTreeParent()) { + if (!content->IsElement()) { + continue; + } + + Element* element = content->AsElement(); if (aGettingFocus) { + if (element->State().HasState(NS_EVENT_STATE_FOCUS_WITHIN)) { + break; + } element->AddStates(NS_EVENT_STATE_FOCUS_WITHIN); } else { element->RemoveStates(NS_EVENT_STATE_FOCUS_WITHIN); @@ -1673,7 +1690,10 @@ nsFocusManager::Blur(nsPIDOMWindowOuter* aWindowToClear, content && content->IsInComposedDoc() && !IsNonFocusableRoot(content); if (content) { if (sendBlurEvent) { - NotifyFocusStateChange(content, shouldShowFocusRing, false); + NotifyFocusStateChange(content, + aContentToFocus, + shouldShowFocusRing, + false); } // if an object/plug-in/remote browser is being blurred, move the system focus @@ -1921,7 +1941,10 @@ nsFocusManager::Focus(nsPIDOMWindowOuter* aWindow, if (aFocusChanged) ScrollIntoView(presShell, aContent, aFlags); - NotifyFocusStateChange(aContent, aWindow->ShouldShowFocusRing(), true); + NotifyFocusStateChange(aContent, + nullptr, + aWindow->ShouldShowFocusRing(), + true); // if this is an object/plug-in/remote browser, focus its widget. Note that we might // no longer be in the same document, due to the events we fired above when diff --git a/dom/base/nsFocusManager.h b/dom/base/nsFocusManager.h index c98b092468..8fe919a155 100644 --- a/dom/base/nsFocusManager.h +++ b/dom/base/nsFocusManager.h @@ -583,6 +583,7 @@ private: // focus rings: in the losing focus case that information could be // wrong.. static void NotifyFocusStateChange(nsIContent* aContent, + nsIContent* aContentToFocus, bool aWindowShouldShowFocusRing, bool aGettingFocus);