From 092434c08c6a0d6381f7f7b57e2cf36febc7601c Mon Sep 17 00:00:00 2001 From: Kate McKinley Date: Thu, 27 Jul 2017 11:01:24 -0700 Subject: [PATCH] Bug 1376651 - Pass the nsIScriptElement instead of allocating a string every time r=ckerschb Change the interface to GetAlowsInline to take an nsISupports* instead of a string, and pass the nsIScriptElement directly. If we don't have an element, then pass nullptr or the mock string created as an nsISupportsString. MozReview-Commit-ID: pgIMxtplsi --HG-- extra : rebase_source : 4691643bb67ff6c78a74a4886a04c4816cff6219 --- dom/base/nsDocument.cpp | 2 +- dom/events/EventListenerManager.cpp | 7 +++- .../security/nsIContentSecurityPolicy.idl | 6 ++-- dom/jsurl/nsJSProtocolHandler.cpp | 2 +- dom/script/ScriptLoader.cpp | 6 +--- dom/security/nsCSPContext.cpp | 33 ++++++++++++++++--- dom/security/test/unit/test_csp_reports.js | 10 ++++-- layout/style/nsStyleUtil.cpp | 8 ++++- 8 files changed, 57 insertions(+), 17 deletions(-) diff --git a/dom/base/nsDocument.cpp b/dom/base/nsDocument.cpp index 1bac64e3bac6..9e9ad0ecf6c2 100644 --- a/dom/base/nsDocument.cpp +++ b/dom/base/nsDocument.cpp @@ -12686,7 +12686,7 @@ nsIDocument::InlineScriptAllowedByCSP() nsresult rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, EmptyString(), // aNonce true, // aParserCreated - EmptyString(), // FIXME get script sample (bug 1314567) + nullptr, // FIXME get script sample (bug 1314567) 0, // aLineNumber &allowsInlineScript); NS_ENSURE_SUCCESS(rv, true); diff --git a/dom/events/EventListenerManager.cpp b/dom/events/EventListenerManager.cpp index 8151fbb69bd2..58915b6ee852 100644 --- a/dom/events/EventListenerManager.cpp +++ b/dom/events/EventListenerManager.cpp @@ -46,6 +46,7 @@ #include "nsIDOMEventListener.h" #include "nsIScriptGlobalObject.h" #include "nsISupports.h" +#include "nsISupportsPrimitives.h" #include "nsIXPConnect.h" #include "nsJSUtils.h" #include "nsNameSpaceManager.h" @@ -876,12 +877,16 @@ EventListenerManager::SetEventHandler(nsIAtom* aName, scriptSample.AppendLiteral(" attribute on "); scriptSample.Append(tagName); scriptSample.AppendLiteral(" element"); + nsCOMPtr sampleIString(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID)); + if (sampleIString) { + sampleIString->SetData(scriptSample); + } bool allowsInlineScript = true; rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, EmptyString(), // aNonce true, // aParserCreated (true because attribute event handler) - scriptSample, + sampleIString, 0, // aLineNumber &allowsInlineScript); NS_ENSURE_SUCCESS(rv, rv); diff --git a/dom/interfaces/security/nsIContentSecurityPolicy.idl b/dom/interfaces/security/nsIContentSecurityPolicy.idl index c362911d4eea..a4e024cedcfd 100644 --- a/dom/interfaces/security/nsIContentSecurityPolicy.idl +++ b/dom/interfaces/security/nsIContentSecurityPolicy.idl @@ -11,6 +11,7 @@ interface nsIDocShell; interface nsIDOMDocument; interface nsIEventTarget; interface nsIPrincipal; +interface nsIScriptElement; interface nsIURI; /** @@ -138,7 +139,8 @@ interface nsIContentSecurityPolicy : nsISerializable * @param aContentPolicyType Either TYPE_SCRIPT or TYPE_STYLESHEET * @param aNonce The nonce string to check against the policy * @param aParserCreated If the script element was created by the HTML Parser - * @param aContent The content of the inline resource to hash + * @param aElementOrContent The script element of the inline resource to hash + * or the content of the psuedo-script to compare to hash * (and compare to the hashes listed in the policy) * @param aLineNumber The line number of the inline resource * (used for reporting) @@ -149,7 +151,7 @@ interface nsIContentSecurityPolicy : nsISerializable boolean getAllowsInline(in nsContentPolicyType aContentPolicyType, in AString aNonce, in boolean aParserCreated, - in AString aContent, + in nsISupports aElementOrContent, in unsigned long aLineNumber); /** diff --git a/dom/jsurl/nsJSProtocolHandler.cpp b/dom/jsurl/nsJSProtocolHandler.cpp index 55f260a724e8..36f2143cab8b 100644 --- a/dom/jsurl/nsJSProtocolHandler.cpp +++ b/dom/jsurl/nsJSProtocolHandler.cpp @@ -182,7 +182,7 @@ nsresult nsJSThunk::EvaluateScript(nsIChannel *aChannel, rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, EmptyString(), // aNonce true, // aParserCreated - EmptyString(), // aContent + nullptr, // aContent 0, // aLineNumber &allowsInlineScript); diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 64804284998d..dc96cdd4cf1a 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -1110,13 +1110,9 @@ CSPAllowsInlineScript(nsIScriptElement* aElement, nsIDocument* aDocument) scriptContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce); bool parserCreated = aElement->GetParserCreated() != mozilla::dom::NOT_FROM_PARSER; - // query the scripttext - nsAutoString scriptText; - aElement->GetScriptText(scriptText); - bool allowInlineScript = false; rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_SCRIPT, - nonce, parserCreated, scriptText, + nonce, parserCreated, aElement, aElement->GetScriptLineNumber(), &allowInlineScript); return allowInlineScript; diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index c158831390a7..c8e87262deb6 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -26,6 +26,7 @@ #include "nsIObserver.h" #include "nsIObserverService.h" #include "nsIStringStream.h" +#include "nsISupportsPrimitives.h" #include "nsIUploadChannel.h" #include "nsIScriptError.h" #include "nsIWebNavigation.h" @@ -505,7 +506,7 @@ NS_IMETHODIMP nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, const nsAString& aNonce, bool aParserCreated, - const nsAString& aContent, + nsISupports* aElementOrContent, uint32_t aLineNumber, bool* outAllowsInline) { @@ -520,12 +521,36 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, return NS_OK; } + nsAutoString content(EmptyString()); + // always iterate all policies, otherwise we might not send out all reports for (uint32_t i = 0; i < mPolicies.Length(); i++) { bool allowed = mPolicies[i]->allows(aContentType, CSP_UNSAFE_INLINE, EmptyString(), aParserCreated) || - mPolicies[i]->allows(aContentType, CSP_NONCE, aNonce, aParserCreated) || - mPolicies[i]->allows(aContentType, CSP_HASH, aContent, aParserCreated); + mPolicies[i]->allows(aContentType, CSP_NONCE, aNonce, aParserCreated); + + // If the inlined script or style is allowed by either unsafe-inline or the + // nonce, go ahead and shortcut this loop so we can avoid allocating + // unecessary strings + if (allowed) { + continue; + } + + // Check the content length to ensure the content is not allocated more than + // once. Even though we are in a for loop, it is probable that there is only one + // policy, so this check may be unnecessary. + if (content.Length() == 0) { + // postpone the allocation until absolutely necessary. + nsCOMPtr stringContent = do_QueryInterface(aElementOrContent); + nsCOMPtr element = do_QueryInterface(aElementOrContent); + if (stringContent) { + Unused << stringContent->GetData(content); + } else if (element) { + element->GetScriptText(content); + } + } + + allowed = mPolicies[i]->allows(aContentType, CSP_HASH, content, aParserCreated); if (!allowed) { // policy is violoated: deny the load unless policy is report only and @@ -537,7 +562,7 @@ nsCSPContext::GetAllowsInline(nsContentPolicyType aContentType, mPolicies[i]->getDirectiveStringForContentType(aContentType, violatedDirective); reportInlineViolation(aContentType, aNonce, - aContent, + content, violatedDirective, i, aLineNumber); diff --git a/dom/security/test/unit/test_csp_reports.js b/dom/security/test/unit/test_csp_reports.js index 6c88fb1e10fb..4bb852581d85 100644 --- a/dom/security/test/unit/test_csp_reports.js +++ b/dom/security/test/unit/test_csp_reports.js @@ -107,6 +107,9 @@ function run_test() { ":" + REPORT_SERVER_PORT + "/foo/self"); + let content = Cc["@mozilla.org/supports-string;1"]. + createInstance(Ci.nsISupportsString); + content.data = ""; // test that inline script violations cause a report. makeTest(0, {"blocked-uri": "self"}, false, function(csp) { @@ -114,7 +117,7 @@ function run_test() { inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT, "", // aNonce false, // aParserCreated - "", // aContent + content, // aContent 0); // aLineNumber // this is not a report only policy, so it better block inline scripts @@ -158,10 +161,13 @@ function run_test() { makeTest(3, {"blocked-uri": "self"}, true, function(csp) { let inlineOK = true; + let content = Cc["@mozilla.org/supports-string;1"]. + createInstance(Ci.nsISupportsString); + content.data = ""; inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT, "", // aNonce false, // aParserCreated - "", // aContent + content, // aContent 0); // aLineNumber // this is a report only policy, so it better allow inline scripts diff --git a/layout/style/nsStyleUtil.cpp b/layout/style/nsStyleUtil.cpp index c919da72c1b7..45990c691ed1 100644 --- a/layout/style/nsStyleUtil.cpp +++ b/layout/style/nsStyleUtil.cpp @@ -15,6 +15,7 @@ #include "nsIContentPolicy.h" #include "nsIContentSecurityPolicy.h" #include "nsIURI.h" +#include "nsISupportsPrimitives.h" #include "nsPrintfCString.h" #include @@ -849,11 +850,16 @@ nsStyleUtil::CSPAllowsInlineStyle(nsIContent* aContent, aContent->GetAttr(kNameSpaceID_None, nsGkAtoms::nonce, nonce); } + nsCOMPtr styleText(do_CreateInstance(NS_SUPPORTS_STRING_CONTRACTID)); + if (styleText) { + styleText->SetData(aStyleText); + } + bool allowInlineStyle = true; rv = csp->GetAllowsInline(nsIContentPolicy::TYPE_STYLESHEET, nonce, false, // aParserCreated only applies to scripts - aStyleText, aLineNumber, + styleText, aLineNumber, &allowInlineStyle); NS_ENSURE_SUCCESS(rv, false);