Changeset 147086 in webkit


Ignore:
Timestamp:
Mar 28, 2013 1:59:00 AM (11 years ago)
Author:
mkwst@chromium.org
Message:

X-Frame-Options: Multiple headers are ignored completely.
​https://bugs.webkit.org/show_bug.cgi?id=113387

Reviewed by Nate Chapin.

Source/WebCore:

If a server sends multiple 'X-Frame-Options' headers, we end up with a
value like 'SAMEORIGIN, SAMEORIGIN'. Currently, we're treating that as
invalid, and ignoring the header. It would be safer to follow Gecko's
lead[1] by:

  • Folding duplicated entries into their common value (that is: 'sameorigin, sameorigin' -> 'sameorigin').
  • Failing closed in the case of conflicts (that is: 'sameorigin, allowall' -> 'deny').

[1]: ​https://bugzilla.mozilla.org/show_bug.cgi?id=761655

Tests: http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html

http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow.html
http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html

  • loader/FrameLoader.cpp:

(WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):

Call out to parseXFrameOptionsHeader to get the header's disposition
and deal with each case in a switch statement for clarity. Add a new
console warning for the conflict case described above.

  • platform/network/HTTPParsers.cpp:

(WebCore::parseXFrameOptionsHeader):

  • platform/network/HTTPParsers.h:

Move X-Frame-Options parsing out into HTTPParsers, as it's getting
more and more complicated. To do this, the patch defines a new enum
to pass around the header's disposition.

LayoutTests:

  • http/tests/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi: Added.
  • http/tests/security/XFrameOptions/resources/x-frame-options-multiple-headers-sameorigin.cgi: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict-expected.txt: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow-expected.txt: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow.html: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny-expected.txt: Added.
  • http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html: Added.
  • platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict-expected.txt: Added.
  • platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow-expected.txt: Added.
  • platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny-expected.txt: Added.
Location:
trunk
Files:
11 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r147084 r147086  
     12013-03-28  Mike West  <mkwst@chromium.org>
     2
     3        X-Frame-Options: Multiple headers are ignored completely.
     4        https://bugs.webkit.org/show_bug.cgi?id=113387
     5
     6        Reviewed by Nate Chapin.
     7
     8        * http/tests/security/XFrameOptions/resources/x-frame-options-multiple-headers-conflict.cgi: Added.
     9        * http/tests/security/XFrameOptions/resources/x-frame-options-multiple-headers-sameorigin.cgi: Added.
     10        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict-expected.txt: Added.
     11        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html: Added.
     12        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow-expected.txt: Added.
     13        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow.html: Added.
     14        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny-expected.txt: Added.
     15        * http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html: Added.
     16        * platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict-expected.txt: Added.
     17        * platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow-expected.txt: Added.
     18        * platform/chromium/http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny-expected.txt: Added.
     19
    1202013-03-28  Mihai Tica  <mitica@adobe.com>
    221

  • trunk/Source/WebCore/ChangeLog

    r147082 r147086  
     12013-03-28  Mike West  <mkwst@chromium.org>
     2
     3        X-Frame-Options: Multiple headers are ignored completely.
     4        https://bugs.webkit.org/show_bug.cgi?id=113387
     5
     6        Reviewed by Nate Chapin.
     7
     8        If a server sends multiple 'X-Frame-Options' headers, we end up with a
     9        value like 'SAMEORIGIN, SAMEORIGIN'. Currently, we're treating that as
     10        invalid, and ignoring the header. It would be safer to follow Gecko's
     11        lead[1] by:
     12
     13        - Folding duplicated entries into their common value (that is:
     14          'sameorigin, sameorigin' -> 'sameorigin').
     15
     16        - Failing closed in the case of conflicts (that is:
     17          'sameorigin, allowall' -> 'deny').
     18
     19        [1]: https://bugzilla.mozilla.org/show_bug.cgi?id=761655
     20
     21        Tests: http/tests/security/XFrameOptions/x-frame-options-multiple-headers-conflict.html
     22               http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-allow.html
     23               http/tests/security/XFrameOptions/x-frame-options-multiple-headers-sameorigin-deny.html
     24
     25        * loader/FrameLoader.cpp:
     26        (WebCore::FrameLoader::shouldInterruptLoadForXFrameOptions):
     27            Call out to parseXFrameOptionsHeader to get the header's disposition
     28            and deal with each case in a switch statement for clarity. Add a new
     29            console warning for the conflict case described above.
     30        * platform/network/HTTPParsers.cpp:
     31        (WebCore::parseXFrameOptionsHeader):
     32        * platform/network/HTTPParsers.h:
     33            Move X-Frame-Options parsing out into HTTPParsers, as it's getting
     34            more and more complicated. To do this, the patch defines a new enum
     35            to pass around the header's disposition.
     36
    1372013-03-28  Mihnea Ovidenie  <mihnea@adobe.com>
    238

  • trunk/Source/WebCore/loader/FrameLoader.cpp

    r146726 r147086  
    29572957        return false;
    29582958
    2959     if (equalIgnoringCase(content, "deny"))
    2960         return true;
    2961     else if (equalIgnoringCase(content, "sameorigin")) {
     2959    XFrameOptionsDisposition disposition = parseXFrameOptionsHeader(content);
     2960
     2961    switch (disposition) {
     2962    case XFrameOptionsSameOrigin: {
    29622963        FeatureObserver::observe(m_frame->document(), FeatureObserver::XFrameOptionsSameOrigin);
    29632964        RefPtr<SecurityOrigin> origin = SecurityOrigin::create(url);
    … …  
    29702971            }
    29712972        }
    2972     } else if (!equalIgnoringCase(content, "allowall")) {
    2973         String message = "Invalid 'X-Frame-Options' header encountered when loading '" + url.elidedString() + "': '" + content + "' is not a recognized directive. The header will be ignored.";
    2974         m_frame->document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, message, requestIdentifier);
    2975     }
    2976 
    2977     return false;
     2973        return false;
     2974    }
     2975    case XFrameOptionsDeny:
     2976        return true;
     2977    case XFrameOptionsAllowAll:
     2978        return false;
     2979    case XFrameOptionsConflict:
     2980        m_frame->document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, "Multiple 'X-Frame-Options' headers with conflicting values ('" + content + "') encountered when loading '" + url.elidedString() + "'. Falling back to 'DENY'.", requestIdentifier);
     2981        return true;
     2982    case XFrameOptionsInvalid:
     2983        m_frame->document()->addConsoleMessage(JSMessageSource, ErrorMessageLevel, "Invalid 'X-Frame-Options' header encountered when loading '" + url.elidedString() + "': '" + content + "' is not a recognized directive. The header will be ignored.", requestIdentifier);
     2984        return false;
     2985    default:
     2986        ASSERT_NOT_REACHED();
     2987        return false;
     2988    }
    29782989}
    29792990

  • trunk/Source/WebCore/platform/network/HTTPParsers.cpp

    r146908 r147086  
    456456}
    457457
     458XFrameOptionsDisposition parseXFrameOptionsHeader(const String& header)
     459{
     460    XFrameOptionsDisposition result = XFrameOptionsNone;
     461
     462    if (header.isEmpty())
     463        return result;
     464
     465    Vector<String> headers;
     466    header.split(',', headers);
     467
     468    for (size_t i = 0; i < headers.size(); i++) {
     469        String currentHeader = headers[i].stripWhiteSpace();
     470        XFrameOptionsDisposition currentValue = XFrameOptionsNone;
     471        if (equalIgnoringCase(currentHeader, "deny"))
     472            currentValue = XFrameOptionsDeny;
     473        else if (equalIgnoringCase(currentHeader, "sameorigin"))
     474            currentValue = XFrameOptionsSameOrigin;
     475        else if (equalIgnoringCase(currentHeader, "allowall"))
     476            currentValue = XFrameOptionsAllowAll;
     477        else
     478            currentValue = XFrameOptionsInvalid;
     479
     480        if (result == XFrameOptionsNone)
     481            result = currentValue;
     482        else if (result != currentValue)
     483            return XFrameOptionsConflict;
     484    }
     485    return result;
     486}
     487
    458488bool parseRange(const String& range, long long& rangeOffset, long long& rangeEnd, long long& rangeSuffixLength)
    459489{

  • trunk/Source/WebCore/platform/network/HTTPParsers.h

    r146908 r147086  
    5555#endif
    5656
     57enum XFrameOptionsDisposition {
     58    XFrameOptionsNone,
     59    XFrameOptionsDeny,
     60    XFrameOptionsSameOrigin,
     61    XFrameOptionsAllowAll,
     62    XFrameOptionsInvalid,
     63    XFrameOptionsConflict
     64};
     65
    5766ContentDispositionType contentDispositionType(const String&);
    5867bool isValidHTTPHeaderValue(const String&);
    … …  
    6675ContentSecurityPolicy::ReflectedXSSDisposition parseXSSProtectionHeader(const String& header, String& failureReason, unsigned& failurePosition, String& reportURL);
    6776String extractReasonPhraseFromHTTPStatusLine(const String&);
     77XFrameOptionsDisposition parseXFrameOptionsHeader(const String&);
    6878
    6979// -1 could be set to one of the return parameters to indicate the value is not specified.
Note: See TracChangeset for help on using the changeset viewer.