Changeset 198936 in webkit


Ignore:
Timestamp:
Mar 31, 2016 6:54:47 PM (8 years ago)
Author:
dbates@webkit.org
Message:

REGRESSION (r197724): <object>/<embed> with no URL does not match source *
​https://bugs.webkit.org/show_bug.cgi?id=156079
<rdar://problem/25470805>

Reviewed by Brent Fulgham.

Source/WebCore:

Fixes an issue where HTML object and embed elements that are not associated with a URL are
allowed to load when object-src/default-src contains source *. More generally, we allow
such elements to load so long as object-src/default-src is not 'none' per section object-src
of the Content Security Policy Level 3 spec., <​http://w3c.github.io/webappsec-csp> (Editor's Draft, 29 February 2016).

Tests: http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star.html

http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star.html
http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star.html
http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star.html

  • page/csp/ContentSecurityPolicy.cpp:

(WebCore::ContentSecurityPolicy::allowObjectFromSource): Modified to call violatedDirectiveInAnyPolicy() passing
ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::Yes.

  • page/csp/ContentSecurityPolicyDirectiveList.cpp:

(WebCore::checkSource): Modified to take argument of type ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone (defaults to false)
and pass it through to ContentSecurityPolicySourceListDirective.
(WebCore::checkFrameAncestors): Explicitly pass ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No
to avoid URL from having the compiler implicitly convert it to a String and selecting override ContentSecurityPolicySourceListDirective::allows(const String&),
which will lead to incorrect results. We will look to make this code less error prone in <​https://bugs.webkit.org/show_bug.cgi?id=156086>.
(WebCore::ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource): Modified to take argument of type
ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone and pass it through.

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

(WebCore::ContentSecurityPolicySourceList::parse): Set instance variable m_isNone to true so that we can differentiate
a source list with value 'none' from a source list that lists one or more sources or non-none keywords.

  • page/csp/ContentSecurityPolicySourceList.h:

(WebCore::ContentSecurityPolicySourceList::isNone): Added.

  • page/csp/ContentSecurityPolicySourceListDirective.cpp:

(WebCore::ContentSecurityPolicySourceListDirective::allows): Modified to take argument of type ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone
and updated code to return true for an empty URL only if this argument is ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::Yes and
the source list does not have value 'none'.

  • page/csp/ContentSecurityPolicySourceListDirective.h:

LayoutTests:

Add tests to ensure that HTML object and embed elements are allowed by source *.

  • platform/ios-simulator/TestExpectations: Skip added tests as plugins are not supported on iOS.
  • http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star.html: Added.
  • http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star.html: Added.
  • http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star.html: Added.
  • http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star-expected.txt: Added.
  • http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star.html: Added.
Location:
trunk
Files:
8 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r198928 r198936  
     12016-03-31  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION (r197724): <object>/<embed> with no URL does not match source *
     4        https://bugs.webkit.org/show_bug.cgi?id=156079
     5        <rdar://problem/25470805>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Add tests to ensure that HTML object and embed elements are allowed by source *.
     10
     11        * platform/ios-simulator/TestExpectations: Skip added tests as plugins are not supported on iOS.
     12        * http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star-expected.txt: Added.
     13        * http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star.html: Added.
     14        * http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star-expected.txt: Added.
     15        * http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star.html: Added.
     16        * http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star-expected.txt: Added.
     17        * http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star.html: Added.
     18        * http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star-expected.txt: Added.
     19        * http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star.html: Added.
     20
    1212016-03-31  Saam barati  <sbarati@apple.com>
    222

  • trunk/LayoutTests/platform/ios-simulator/TestExpectations

    r198910 r198936  
    8888http/tests/security/contentSecurityPolicy/1.1/plugintypes-url-01.html
    8989http/tests/security/contentSecurityPolicy/1.1/plugintypes-url-02.html
     90http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star.html
     91http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star.html
    9092http/tests/security/contentSecurityPolicy/object-src-param-code-blocked.html
    9193http/tests/security/contentSecurityPolicy/object-src-param-movie-blocked.html
    9294http/tests/security/contentSecurityPolicy/object-src-param-src-blocked.html
    9395http/tests/security/contentSecurityPolicy/object-src-param-url-blocked.html
     96http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star.html
     97http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star.html
    9498
    9599# Pointer-lock not supported on iOS

  • trunk/Source/WebCore/ChangeLog

    r198932 r198936  
     12016-03-31  Daniel Bates  <dabates@apple.com>
     2
     3        REGRESSION (r197724): <object>/<embed> with no URL does not match source *
     4        https://bugs.webkit.org/show_bug.cgi?id=156079
     5        <rdar://problem/25470805>
     6
     7        Reviewed by Brent Fulgham.
     8
     9        Fixes an issue where HTML object and embed elements that are not associated with a URL are
     10        allowed to load when object-src/default-src contains source *. More generally, we allow
     11        such elements to load so long as object-src/default-src is not 'none' per section object-src
     12        of the Content Security Policy Level 3 spec., <http://w3c.github.io/webappsec-csp> (Editor's Draft, 29 February 2016).
     13
     14        Tests: http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-default-src-star.html
     15               http/tests/security/contentSecurityPolicy/embed-with-no-url-allowed-by-star.html
     16               http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-default-src-star.html
     17               http/tests/security/contentSecurityPolicy/object-with-no-url-allowed-by-star.html
     18
     19        * page/csp/ContentSecurityPolicy.cpp:
     20        (WebCore::ContentSecurityPolicy::allowObjectFromSource): Modified to call violatedDirectiveInAnyPolicy() passing
     21        ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::Yes.
     22        * page/csp/ContentSecurityPolicyDirectiveList.cpp:
     23        (WebCore::checkSource): Modified to take argument of type ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone (defaults to false)
     24        and pass it through to ContentSecurityPolicySourceListDirective.
     25        (WebCore::checkFrameAncestors): Explicitly pass ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No
     26        to avoid URL from having the compiler implicitly convert it to a String and selecting override ContentSecurityPolicySourceListDirective::allows(const String&),
     27        which will lead to incorrect results. We will look to make this code less error prone in <https://bugs.webkit.org/show_bug.cgi?id=156086>.
     28        (WebCore::ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource): Modified to take argument of type
     29        ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone and pass it through.
     30        * page/csp/ContentSecurityPolicyDirectiveList.h:
     31        * page/csp/ContentSecurityPolicySourceList.cpp:
     32        (WebCore::ContentSecurityPolicySourceList::parse): Set instance variable m_isNone to true so that we can differentiate
     33        a source list with value 'none' from a source list that lists one or more sources or non-none keywords.
     34        * page/csp/ContentSecurityPolicySourceList.h:
     35        (WebCore::ContentSecurityPolicySourceList::isNone): Added.
     36        * page/csp/ContentSecurityPolicySourceListDirective.cpp:
     37        (WebCore::ContentSecurityPolicySourceListDirective::allows): Modified to take argument of type ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone
     38        and updated code to return true for an empty URL only if this argument is ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::Yes and
     39        the source list does not have value 'none'.
     40        * page/csp/ContentSecurityPolicySourceListDirective.h:
     41
    1422016-03-31  Saam barati  <sbarati@apple.com>
    243

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

    r198657 r198936  
    386386    if (SchemeRegistry::schemeShouldBypassContentSecurityPolicy(url.protocol()))
    387387        return true;
    388     const ContentSecurityPolicyDirective* violatedDirective = violatedDirectiveInAnyPolicy(&ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource, url);
     388    // As per section object-src of the Content Security Policy Level 3 spec., <http://w3c.github.io/webappsec-csp> (Editor's Draft, 29 February 2016),
     389    // "If plugin content is loaded without an associated URL (perhaps an object element lacks a data attribute, but loads some default plugin based
     390    // on the specified type), it MUST be blocked if object-src's value is 'none', but will otherwise be allowed".
     391    const ContentSecurityPolicyDirective* violatedDirective = violatedDirectiveInAnyPolicy(&ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource, url, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::Yes);
    389392    if (!violatedDirective)
    390393        return true;

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

    r198657 r198936  
    6262}
    6363
    64 static inline bool checkSource(ContentSecurityPolicySourceListDirective* directive, const URL& url)
    65 {
    66     return !directive || directive->allows(url);
     64static inline bool checkSource(ContentSecurityPolicySourceListDirective* directive, const URL& url, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone shouldAllowEmptyURLIfSourceListEmpty = ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No)
     65{
     66    return !directive || directive->allows(url, shouldAllowEmptyURLIfSourceListEmpty);
    6767}
    6868
    … …  
    8282        return true;
    8383    for (Frame* current = frame.tree().parent(); current; current = current->tree().parent()) {
    84         if (!directive->allows(current->document()->url()))
     84        if (!directive->allows(current->document()->url(), ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone::No))
    8585            return false;
    8686    }
    … …  
    258258}
    259259
    260 const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource(const URL& url) const
     260const ContentSecurityPolicyDirective* ContentSecurityPolicyDirectiveList::violatedDirectiveForObjectSource(const URL& url, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone shouldAllowEmptyURLIfSourceListEmpty) const
    261261{
    262262    if (url.isBlankURL())
    263263        return nullptr;
    264264    ContentSecurityPolicySourceListDirective* operativeDirective = this->operativeDirective(m_objectSrc.get());
    265     if (checkSource(operativeDirective, url))
     265    if (checkSource(operativeDirective, url, shouldAllowEmptyURLIfSourceListEmpty))
    266266        return nullptr;
    267267    return operativeDirective;

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

    r198657 r198936  
    6666    const ContentSecurityPolicyDirective* violatedDirectiveForImage(const URL&) const;
    6767    const ContentSecurityPolicyDirective* violatedDirectiveForMedia(const URL&) const;
    68     const ContentSecurityPolicyDirective* violatedDirectiveForObjectSource(const URL&) const;
     68    const ContentSecurityPolicyDirective* violatedDirectiveForObjectSource(const URL&, ContentSecurityPolicySourceListDirective::ShouldAllowEmptyURLIfSourceListIsNotNone) const;
    6969    const ContentSecurityPolicyDirective* violatedDirectiveForPluginType(const String& type, const String& typeAttribute) const;
    7070    const ContentSecurityPolicyDirective* violatedDirectiveForScript(const URL&) const;

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

    r198657 r198936  
    120120void ContentSecurityPolicySourceList::parse(const String& value)
    121121{
    122     // We represent 'none' as an empty m_list.
    123     if (isSourceListNone(value))
     122    if (isSourceListNone(value)) {
     123        m_isNone = true;
    124124        return;
     125    }
    125126    auto characters = StringView(value).upconvertedCharacters();
    126127    parse(characters, characters + value.length());

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

    r197944 r198936  
    5555    bool allowEval() const { return m_allowEval; }
    5656    bool allowSelf() const { return m_allowSelf; }
     57    bool isNone() const { return m_isNone; }
    5758
    5859private:
    … …  
    8182    bool m_allowInline { false };
    8283    bool m_allowEval { false };
     84    bool m_isNone { false };
    8385};
    8486

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

    r198657 r198936  
    4141}
    4242
    43 bool ContentSecurityPolicySourceListDirective::allows(const URL& url)
     43bool ContentSecurityPolicySourceListDirective::allows(const URL& url, ShouldAllowEmptyURLIfSourceListIsNotNone shouldAllowEmptyURLIfSourceListEmpty)
    4444{
    45     // FIXME: We should investigate returning false for an empty URL.
    4645    if (url.isEmpty())
    47         return m_sourceList.allowSelf();
     46        return shouldAllowEmptyURLIfSourceListEmpty == ShouldAllowEmptyURLIfSourceListIsNotNone::Yes && !m_sourceList.isNone();
    4847    return m_sourceList.matches(url);
    4948}

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

    r198657 r198936  
    3939    ContentSecurityPolicySourceListDirective(const ContentSecurityPolicyDirectiveList&, const String& name, const String& value);
    4040
    41     bool allows(const URL&);
     41    enum class ShouldAllowEmptyURLIfSourceListIsNotNone { No, Yes };
     42    bool allows(const URL&, ShouldAllowEmptyURLIfSourceListIsNotNone);
    4243    bool allows(const ContentSecurityPolicyHash&) const;
    4344    bool allows(const String& nonce) const;
Note: See TracChangeset for help on using the changeset viewer.