CSP should only notify Inspector to pause the debugger on the first policy to violate...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 May 2018 17:41:50 +0000 (17:41 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 7 May 2018 17:41:50 +0000 (17:41 +0000)
https://bugs.webkit.org/show_bug.cgi?id=185364

Reviewed by Brent Fulgham.

Notify Web Inspector that a script was blocked on the first enforced CSP policy that it
violates.

A page can have more than one enforced Content Security Policy. Currently for inline
scripts, inline event handlers, JavaScript URLs, and eval() that are blocked by CSP
we notify Web Inspector that it was blocked for each CSP policy that blocked it. When
Web Inspector is notified it pauses script execution. It does not seem very meaningful
to pause script execution on the same script for each CSP policy that blocked it.
Therefore, only tell Web Inspector that a script was blocked for the first enforced CSP
policy that blocked it.

* page/csp/ContentSecurityPolicy.cpp:
(WebCore::ContentSecurityPolicy::allowJavaScriptURLs const):
(WebCore::ContentSecurityPolicy::allowInlineEventHandlers const):
(WebCore::ContentSecurityPolicy::allowInlineScript const):
(WebCore::ContentSecurityPolicy::allowEval const):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@231443 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/page/csp/ContentSecurityPolicy.cpp

index 690ecb9..3ce72cf 100644 (file)
@@ -1,5 +1,29 @@
 2018-05-07  Daniel Bates  <dabates@apple.com>
 
+        CSP should only notify Inspector to pause the debugger on the first policy to violate a directive
+        https://bugs.webkit.org/show_bug.cgi?id=185364
+
+        Reviewed by Brent Fulgham.
+
+        Notify Web Inspector that a script was blocked on the first enforced CSP policy that it
+        violates.
+
+        A page can have more than one enforced Content Security Policy. Currently for inline
+        scripts, inline event handlers, JavaScript URLs, and eval() that are blocked by CSP
+        we notify Web Inspector that it was blocked for each CSP policy that blocked it. When
+        Web Inspector is notified it pauses script execution. It does not seem very meaningful
+        to pause script execution on the same script for each CSP policy that blocked it.
+        Therefore, only tell Web Inspector that a script was blocked for the first enforced CSP
+        policy that blocked it.
+
+        * page/csp/ContentSecurityPolicy.cpp:
+        (WebCore::ContentSecurityPolicy::allowJavaScriptURLs const):
+        (WebCore::ContentSecurityPolicy::allowInlineEventHandlers const):
+        (WebCore::ContentSecurityPolicy::allowInlineScript const):
+        (WebCore::ContentSecurityPolicy::allowEval const):
+
+2018-05-07  Daniel Bates  <dabates@apple.com>
+
         Substitute CrossOriginPreflightResultCache::clear() for CrossOriginPreflightResultCache::empty()
         https://bugs.webkit.org/show_bug.cgi?id=185170
 
index fc2fd12..b579e96 100644 (file)
@@ -351,11 +351,14 @@ bool ContentSecurityPolicy::allowJavaScriptURLs(const String& contextURL, const
 {
     if (overrideContentSecurityPolicy)
         return true;
+    bool didNotifyInspector = false;
     auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
         String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), "Refused to execute a script", "its hash, its nonce, or 'unsafe-inline'");
         reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), consoleMessage, contextURL, TextPosition(contextLine, WTF::OrdinalNumber()));
-        if (!violatedDirective.directiveList().isReportOnly())
+        if (!didNotifyInspector && violatedDirective.directiveList().isReportOnly()) {
             reportBlockedScriptExecutionToInspector(violatedDirective.text());
+            didNotifyInspector = true;
+        }
     };
     return allPoliciesAllow(WTFMove(handleViolatedDirective), &ContentSecurityPolicyDirectiveList::violatedDirectiveForUnsafeInlineScript);
 }
@@ -364,11 +367,14 @@ bool ContentSecurityPolicy::allowInlineEventHandlers(const String& contextURL, c
 {
     if (overrideContentSecurityPolicy)
         return true;
+    bool didNotifyInspector = false;
     auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
         String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), "Refused to execute a script for an inline event handler", "'unsafe-inline'");
         reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), consoleMessage, contextURL, TextPosition(contextLine, WTF::OrdinalNumber()));
-        if (!violatedDirective.directiveList().isReportOnly())
+        if (!didNotifyInspector && !violatedDirective.directiveList().isReportOnly()) {
             reportBlockedScriptExecutionToInspector(violatedDirective.text());
+            didNotifyInspector = true;
+        }
     };
     return allPoliciesAllow(WTFMove(handleViolatedDirective), &ContentSecurityPolicyDirectiveList::violatedDirectiveForUnsafeInlineScript);
 }
@@ -399,6 +405,7 @@ bool ContentSecurityPolicy::allowInlineScript(const String& contextURL, const WT
 {
     if (overrideContentSecurityPolicy)
         return true;
+    bool didNotifyInspector = false;
     bool foundHashInEnforcedPolicies;
     bool foundHashInReportOnlyPolicies;
     std::tie(foundHashInEnforcedPolicies, foundHashInReportOnlyPolicies) = findHashOfContentInPolicies(&ContentSecurityPolicyDirectiveList::violatedDirectiveForScriptHash, scriptContent, m_hashAlgorithmsForInlineScripts);
@@ -407,8 +414,10 @@ bool ContentSecurityPolicy::allowInlineScript(const String& contextURL, const WT
     auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
         String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), "Refused to execute a script", "its hash, its nonce, or 'unsafe-inline'");
         reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), consoleMessage, contextURL, TextPosition(contextLine, WTF::OrdinalNumber()));
-        if (!violatedDirective.directiveList().isReportOnly())
+        if (!didNotifyInspector && !violatedDirective.directiveList().isReportOnly()) {
             reportBlockedScriptExecutionToInspector(violatedDirective.text());
+            didNotifyInspector = true;
+        }
     };
     // FIXME: We should not report that the inline script violated a policy when its hash matched a source
     // expression in the policy and the page has more than one policy. See <https://bugs.webkit.org/show_bug.cgi?id=159832>.
@@ -443,11 +452,14 @@ bool ContentSecurityPolicy::allowEval(JSC::ExecState* state, bool overrideConten
 {
     if (overrideContentSecurityPolicy)
         return true;
+    bool didNotifyInspector = false;
     auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
         String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), "Refused to execute a script", "'unsafe-eval'");
         reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, URL(), consoleMessage, state);
-        if (!violatedDirective.directiveList().isReportOnly())
+        if (!didNotifyInspector && !violatedDirective.directiveList().isReportOnly()) {
             reportBlockedScriptExecutionToInspector(violatedDirective.text());
+            didNotifyInspector = true;
+        }
     };
     return allPoliciesAllow(WTFMove(handleViolatedDirective), &ContentSecurityPolicyDirectiveList::violatedDirectiveForUnsafeEval);
 }