⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 287783 in webkit


Ignore:
Timestamp:
Jan 7, 2022, 2:22:38 PM (5 years ago)
Author:
Kate Cheney
Message:

CSP: strict-dynamic is causing incorrect and unexpected behavior
​https://bugs.webkit.org/show_bug.cgi?id=234756
<rdar://problem/87018316>

Reviewed by Brent Fulgham.

Source/WebCore:

Per the CSP spec, if strict-dynamic is included in the script-src
directive, 'self' and 'unsafe-inline' should be ignored. This fixes a
bug where they were only ignored if specified before 'strict-dynamic'.

Additionally, this reports the sourceURL as empty for inline scripts
instead of using the contextURL, which was unexpectedly allowing
inline scripts when "self" was used with "strict-dynamic".

Tests: http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list.html

http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list.html
http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self.html
http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline.html

  • dom/ScriptElement.cpp:

(WebCore::ScriptElement::requestClassicScript):
(WebCore::ScriptElement::executeClassicScript):

  • page/csp/ContentSecurityPolicy.cpp:

(WebCore::ContentSecurityPolicy::allScriptPoliciesAllow const):
(WebCore::ContentSecurityPolicy::allowNonParserInsertedScripts const):

  • page/csp/ContentSecurityPolicy.h:
  • page/csp/ContentSecurityPolicySourceList.cpp:

(WebCore::ContentSecurityPolicySourceList::parseSource):

LayoutTests:

Add tests with re-arranged ordering of the source expressions.

  • http/tests/security/contentSecurityPolicy/resources/simpleSourcedScript.js: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list.html: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list.html: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self.html: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline.html: Added.
Location:
trunk
Files:
9 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r287780 r287783  
     12022-01-07  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        CSP: strict-dynamic is causing incorrect and unexpected behavior
     4        https://bugs.webkit.org/show_bug.cgi?id=234756
     5        <rdar://problem/87018316>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Add tests with re-arranged ordering of the source expressions.
     10
     11        * http/tests/security/contentSecurityPolicy/resources/simpleSourcedScript.js: Added.
     12        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list-expected.txt: Added.
     13        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list.html: Added.
     14        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list-expected.txt: Added.
     15        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list.html: Added.
     16        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self-expected.txt: Added.
     17        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self.html: Added.
     18        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline-expected.txt: Added.
     19        * http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline.html: Added.
     20
    1212022-01-07  Chris Dumez  <cdumez@apple.com>
    222

  • trunk/LayoutTests/http/tests/security/contentSecurityPolicy/strict-dynamic-module-script-expected.txt

    r287756 r287783  
    1 CONSOLE MESSAGE: Refused to load http://127.0.0.1:8000/security/contentSecurityPolicy/strict-dynamic-module-script.html because it does not appear in the script-src directive of the Content Security Policy.
     1CONSOLE MESSAGE: Refused to execute a script because it does not appear in the script-src directive of the Content Security Policy.
    22
    33PASS All the expected CSP violation reports have been fired.

  • trunk/Source/WebCore/ChangeLog

    r287782 r287783  
     12022-01-07  Kate Cheney  <katherine_cheney@apple.com>
     2
     3        CSP: strict-dynamic is causing incorrect and unexpected behavior
     4        https://bugs.webkit.org/show_bug.cgi?id=234756
     5        <rdar://problem/87018316>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Per the CSP spec, if strict-dynamic is included in the script-src
     10        directive, 'self' and 'unsafe-inline' should be ignored. This fixes a
     11        bug where they were only ignored if specified before 'strict-dynamic'.
     12
     13        Additionally, this reports the sourceURL as empty for inline scripts
     14        instead of using the contextURL, which was unexpectedly allowing
     15        inline scripts when "self" was used with "strict-dynamic".
     16
     17        Tests: http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-host-list.html
     18               http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-scheme-list.html
     19               http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-self.html
     20               http/tests/security/contentSecurityPolicy/strict-dynamic-mixed-order-unsafe-inline.html
     21
     22        * dom/ScriptElement.cpp:
     23        (WebCore::ScriptElement::requestClassicScript):
     24        (WebCore::ScriptElement::executeClassicScript):
     25        * page/csp/ContentSecurityPolicy.cpp:
     26        (WebCore::ContentSecurityPolicy::allScriptPoliciesAllow const):
     27        (WebCore::ContentSecurityPolicy::allowNonParserInsertedScripts const):
     28        * page/csp/ContentSecurityPolicy.h:
     29        * page/csp/ContentSecurityPolicySourceList.cpp:
     30        (WebCore::ContentSecurityPolicySourceList::parseSource):
     31
    1322022-01-07  Said Abou-Hallawa  <said@apple.com>
    233

  • trunk/Source/WebCore/dom/ScriptElement.cpp

    r287756 r287783  
    305305
    306306        const auto& contentSecurityPolicy = *m_element.document().contentSecurityPolicy();
    307         if (!contentSecurityPolicy.allowNonParserInsertedScripts(scriptURL, m_startLineNumber, m_element.nonce(), String(), m_parserInserted))
     307        if (!contentSecurityPolicy.allowNonParserInsertedScripts(scriptURL, URL(), m_startLineNumber, m_element.nonce(), String(), m_parserInserted))
    308308            return false;
    309309
    … …  
    377377    ASSERT(m_element.document().contentSecurityPolicy());
    378378    const auto& contentSecurityPolicy = *m_element.document().contentSecurityPolicy();
    379     if (!contentSecurityPolicy.allowNonParserInsertedScripts(m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted))
     379    if (!contentSecurityPolicy.allowNonParserInsertedScripts(URL(), m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted))
    380380        return false;
    381381
    … …  
    401401        ASSERT(m_element.document().contentSecurityPolicy());
    402402        const ContentSecurityPolicy& contentSecurityPolicy = *m_element.document().contentSecurityPolicy();
    403         if (!contentSecurityPolicy.allowNonParserInsertedScripts(m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted))
     403        if (!contentSecurityPolicy.allowNonParserInsertedScripts(URL(), m_element.document().url(), m_startLineNumber, m_element.nonce(), sourceCode.source(), m_parserInserted))
    404404            return;
    405405

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.cpp

    r287756 r287783  
    460460}
    461461
    462 bool ContentSecurityPolicy::allowNonParserInsertedScripts(const URL& url, const OrdinalNumber& contextLine, const String& nonce, const StringView& scriptContent, ParserInserted parserInserted) const
     462bool ContentSecurityPolicy::allowNonParserInsertedScripts(const URL& sourceURL, const URL& contextURL, const OrdinalNumber& contextLine, const String& nonce, const StringView& scriptContent, ParserInserted parserInserted) const
    463463{
    464464    if (!shouldPerformEarlyCSPCheck())
    … …  
    467467    auto handleViolatedDirective = [&] (const ContentSecurityPolicyDirective& violatedDirective) {
    468468        TextPosition sourcePosition(contextLine, OrdinalNumber());
    469         String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, url, "Refused to load");
    470         reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrcElem, violatedDirective, url.string(), consoleMessage, String(), scriptContent, sourcePosition);
    471     };
    472 
    473     return allScriptPoliciesAllow(handleViolatedDirective, url, nonce, scriptContent, parserInserted);
     469        const char* message = sourceURL.isEmpty() ? "Refused to execute a script" : "Refused to load";
     470        String consoleMessage = consoleMessageForViolation(ContentSecurityPolicyDirectiveNames::scriptSrc, violatedDirective, sourceURL, message);
     471        reportViolation(ContentSecurityPolicyDirectiveNames::scriptSrcElem, violatedDirective, sourceURL.string(), consoleMessage, contextURL.string(), scriptContent, sourcePosition);
     472    };
     473
     474    return allScriptPoliciesAllow(handleViolatedDirective, sourceURL, nonce, scriptContent, parserInserted);
    474475}
    475476

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicy.h

    r287756 r287783  
    102102    bool allowInlineEventHandlers(const String& contextURL, const OrdinalNumber& contextLine, const String& code, Element*, bool overrideContentSecurityPolicy = false) const;
    103103    bool allowInlineScript(const String& contextURL, const OrdinalNumber& contextLine, StringView scriptContent, Element&, bool overrideContentSecurityPolicy = false) const;
    104     bool allowNonParserInsertedScripts(const URL&, const OrdinalNumber&, const String&, const StringView&, ParserInserted) const;
     104    bool allowNonParserInsertedScripts(const URL& sourceURL, const URL& contextURL, const OrdinalNumber&, const String&, const StringView&, ParserInserted) const;
    105105    bool allowInlineStyle(const String& contextURL, const OrdinalNumber& contextLine, StringView styleContent, CheckUnsafeHashes, Element&, bool overrideContentSecurityPolicy = false) const;
    106106

  • trunk/Source/WebCore/page/csp/ContentSecurityPolicySourceList.cpp

    r286853 r287783  
    235235    }
    236236
    237     if (skipExactlyIgnoringASCIICase(buffer, "'strict-dynamic'")) {
     237    if (skipExactlyIgnoringASCIICase(buffer, "'strict-dynamic'") && (m_directiveName == ContentSecurityPolicyDirectiveNames::scriptSrc || m_directiveName == ContentSecurityPolicyDirectiveNames::scriptSrcElem)) {
    238238        m_allowNonParserInsertedScripts = true;
    239239        m_allowSelf = false;
    … …  
    243243
    244244    if (skipExactlyIgnoringASCIICase(buffer, "'self'")) {
    245         m_allowSelf = true;
     245        m_allowSelf = !m_allowNonParserInsertedScripts;
    246246        return source;
    247247    }
    248248
    249249    if (skipExactlyIgnoringASCIICase(buffer, "'unsafe-inline'")) {
    250         m_allowInline = true;
     250        m_allowInline = !m_allowNonParserInsertedScripts;
    251251        return source;
    252252    }
    … …  
    266266        return source;
    267267    }
     268
     269    if (m_allowNonParserInsertedScripts)
     270        return source;
    268271
    269272    auto begin = buffer.position();
Note: See TracChangeset for help on using the changeset viewer.