From dc5571eefd8247b14edbee33c8b881d6b9e97e96 Mon Sep 17 00:00:00 2001 From: "Matt A. Tobin" Date: Wed, 5 Oct 2022 17:59:56 -0500 Subject: [Multi:Sec] Be more explicit about CSP checks and reports. --- components/htmlfive/nsHtml5TreeOpExecutor.cpp | 56 +++++++++++++++++++++- dom/html/HTMLFormElement.cpp | 3 +- dom/html/HTMLSharedElement.cpp | 3 +- system/security/content/nsCSPContext.cpp | 7 +-- .../security/content/nsIContentSecurityPolicy.idl | 11 +++-- 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/components/htmlfive/nsHtml5TreeOpExecutor.cpp b/components/htmlfive/nsHtml5TreeOpExecutor.cpp index 8cf22d804..c79af5321 100644 --- a/components/htmlfive/nsHtml5TreeOpExecutor.cpp +++ b/components/htmlfive/nsHtml5TreeOpExecutor.cpp @@ -1019,9 +1019,63 @@ nsHtml5TreeOpExecutor::SetSpeculationBase(const nsAString& aURL) return; } const nsCString& charset = mDocument->GetDocumentCharacterSet(); - DebugOnly rv = NS_NewURI(getter_AddRefs(mSpeculationBaseURI), aURL, + nsCOMPtr newBaseURI; + nsresult rv = NS_NewURI(getter_AddRefs(newBaseURI), aURL, charset.get(), mDocument->GetDocumentURI()); NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "Failed to create a URI"); + if (!newBaseURI) { + return; + } + + if (!CSPService::sCSPEnabled) { + // If CSP is not enabled, just pass back the URI + mSpeculationBaseURI = newBaseURI; + return; + } + + NS_ASSERTION(NS_IsMainThread(), "Wrong thread!"); + + nsCOMPtr principal = mDocument->NodePrincipal(); + nsCOMPtr domDoc = do_QueryInterface(mDocument); + + // Check the document's CSP usually delivered via the CSP header. + nsCOMPtr documentCsp; + rv = principal->EnsureCSP(domDoc, getter_AddRefs(documentCsp)); + NS_ENSURE_SUCCESS_VOID(rv); + if (documentCsp) { + // base-uri should not fallback to the default-src and preloads should not + // trigger violation reports. + bool cspPermitsBaseURI = true; + rv = documentCsp->Permits( + newBaseURI, + nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, + true /* aSpecific */, + false /* aSendViolationReports */, + &cspPermitsBaseURI); + if (NS_FAILED(rv) || !cspPermitsBaseURI) { + return; + } + } + + // Also check the CSP discovered from the tag during speculative + // parsing. + nsCOMPtr preloadCsp; + rv = principal->EnsurePreloadCSP(domDoc, getter_AddRefs(preloadCsp)); + NS_ENSURE_SUCCESS_VOID(rv); + if (preloadCsp) { + bool cspPermitsBaseURI = true; + rv = preloadCsp->Permits( + newBaseURI, + nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, + true /* aSpecific */, + false /* aSendViolationReports */, + &cspPermitsBaseURI); + if (NS_FAILED(rv) || !cspPermitsBaseURI) { + return; + } + } + + mSpeculationBaseURI = newBaseURI; } void diff --git a/dom/html/HTMLFormElement.cpp b/dom/html/HTMLFormElement.cpp index 2fe452bcd..c8bb52637 100644 --- a/dom/html/HTMLFormElement.cpp +++ b/dom/html/HTMLFormElement.cpp @@ -1721,7 +1721,8 @@ HTMLFormElement::GetActionURL(nsIURI** aActionURL, // policy - do *not* consult default-src, see: // http://www.w3.org/TR/CSP2/#directive-default-src rv = csp->Permits(actionURL, nsIContentSecurityPolicy::FORM_ACTION_DIRECTIVE, - true, &permitsFormAction); + true /*aSpecific */, true /* aSendViolationReports */, + &permitsFormAction); NS_ENSURE_SUCCESS(rv, rv); if (!permitsFormAction) { return NS_ERROR_CSP_FORM_ACTION_VIOLATION; diff --git a/dom/html/HTMLSharedElement.cpp b/dom/html/HTMLSharedElement.cpp index e8c75f8aa..90f9ff62e 100644 --- a/dom/html/HTMLSharedElement.cpp +++ b/dom/html/HTMLSharedElement.cpp @@ -191,7 +191,8 @@ SetBaseURIUsingFirstBaseWithHref(nsIDocument* aDocument, nsIContent* aMustMatch) // http://www.w3.org/TR/CSP2/#directive-default-src bool cspPermitsBaseURI = true; rv = csp->Permits(newBaseURI, nsIContentSecurityPolicy::BASE_URI_DIRECTIVE, - true, &cspPermitsBaseURI); + true /* aSpecific */, true /* aSendViolationReports */, + &cspPermitsBaseURI); if (NS_FAILED(rv) || !cspPermitsBaseURI) { newBaseURI = nullptr; } diff --git a/system/security/content/nsCSPContext.cpp b/system/security/content/nsCSPContext.cpp index ec698b91d..544fe118b 100644 --- a/system/security/content/nsCSPContext.cpp +++ b/system/security/content/nsCSPContext.cpp @@ -1309,6 +1309,7 @@ NS_IMETHODIMP nsCSPContext::Permits(nsIURI* aURI, CSPDirective aDir, bool aSpecific, + bool aSendViolationReports, bool* outPermits) { // Can't perform check without aURI @@ -1323,13 +1324,13 @@ nsCSPContext::Permits(nsIURI* aURI, false, // not redirected. false, // not a preload. aSpecific, - true, // send violation reports + aSendViolationReports, true, // send blocked URI in violation reports false); // not parser created if (CSPCONTEXTLOGENABLED()) { - CSPCONTEXTLOG(("nsCSPContext::Permits, aUri: %s, aDir: %d, isAllowed: %s", - aURI->GetSpecOrDefault().get(), aDir, + CSPCONTEXTLOG(("nsCSPContext::Permits, aUri: %s, aDir: %s, isAllowed: %s", + aURI->GetSpecOrDefault().get(), CSP_CSPDirectiveToString(aDir), *outPermits ? "allow" : "deny")); } diff --git a/system/security/content/nsIContentSecurityPolicy.idl b/system/security/content/nsIContentSecurityPolicy.idl index da4297f33..e76c39c44 100644 --- a/system/security/content/nsIContentSecurityPolicy.idl +++ b/system/security/content/nsIContentSecurityPolicy.idl @@ -252,9 +252,6 @@ interface nsIContentSecurityPolicy : nsISerializable /** * Checks if a specific directive permits loading of a URI. * - * NOTE: Calls to this may trigger violation reports when queried, so the - * return value should not be cached. - * * @param aURI * The URI about to be loaded or used. * @param aDir @@ -266,11 +263,17 @@ interface nsIContentSecurityPolicy : nsISerializable * "false" allows CSP to fall back to default-src. This function * behaves the same for both values of canUseDefault when querying * directives that don't fall-back. + * @param aSendViolationReports + * If `true` and the uri is not allowed then trigger violation reports. + * This should be `false` for caching or preloads. * @return * Whether or not the provided URI is allowed by CSP under the given * directive. (block the pending operation if false). */ - boolean permits(in nsIURI aURI, in CSPDirective aDir, in boolean aSpecific); + boolean permits(in nsIURI aURI, + in CSPDirective aDir, + in boolean aSpecific, + in boolean aSendViolationReports); /** * Delegate method called by the service when sub-elements of the protected -- cgit v1.2.3