Changeset 258631 in webkit


Ignore:
Timestamp:
Mar 18, 2020 7:49:37 AM (4 years ago)
Author:
youenn@apple.com
Message:

Make sure a preflight fails if response headers are invalid
​https://bugs.webkit.org/show_bug.cgi?id=208924

Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

  • web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt: Added.
  • web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html: Added.
  • web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js: Added.

(corsPreflightResponseValidation):

  • web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt: Added.
  • web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html: Added.

Source/WebCore:

Implement ​https://fetch.spec.whatwg.org/#cors-preflight-fetch-0 step 7.3.
In case header parsing is wrong, fail the preflight with a meaningful message.
Update parsing of headers to return an Optional so that parsing error is handled as a nullopt.
Minor refactoring to return Expected/Optional for error handlng instead of passing an out parameter.
Also, adding preflight cache entry if it is valid, no matter whether preflight succeeds or not.

Tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html

imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html

  • loader/CrossOriginAccessControl.cpp:

(WebCore::validatePreflightResponse):

  • loader/CrossOriginPreflightResultCache.cpp:

(WebCore::CrossOriginPreflightResultCacheItem::create):
(WebCore::CrossOriginPreflightResultCacheItem::validateMethodAndHeaders const):

  • loader/CrossOriginPreflightResultCache.h:

(WebCore::CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem):

  • platform/network/HTTPParsers.h:

(WebCore::parseAccessControlAllowList):

  • platform/network/ResourceResponseBase.cpp:

(WebCore::ResourceResponseBase::filter):
(WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):

Location:
trunk
Files:
5 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r258553 r258631  
     12020-03-18  youenn fablet  <youenn@apple.com>
     2
     3        Make sure a preflight fails if response headers are invalid
     4        https://bugs.webkit.org/show_bug.cgi?id=208924
     5
     6        Reviewed by Alex Christensen.
     7
     8        * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any-expected.txt: Added.
     9        * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html: Added.
     10        * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.js: Added.
     11        (corsPreflightResponseValidation):
     12        * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker-expected.txt: Added.
     13        * web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html: Added.
     14
    1152020-03-17  Frederic Wang  <fwang@igalia.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r258630 r258631  
     12020-03-18  youenn fablet  <youenn@apple.com>
     2
     3        Make sure a preflight fails if response headers are invalid
     4        https://bugs.webkit.org/show_bug.cgi?id=208924
     5
     6        Reviewed by Alex Christensen.
     7
     8        Implement https://fetch.spec.whatwg.org/#cors-preflight-fetch-0 step 7.3.
     9        In case header parsing is wrong, fail the preflight with a meaningful message.
     10        Update parsing of headers to return an Optional so that parsing error is handled as a nullopt.
     11        Minor refactoring to return Expected/Optional for error handlng instead of passing an out parameter.
     12        Also, adding preflight cache entry if it is valid, no matter whether preflight succeeds or not.
     13
     14        Tests: imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.html
     15               imported/w3c/web-platform-tests/fetch/api/cors/cors-preflight-response-validation.any.worker.html
     16
     17        * loader/CrossOriginAccessControl.cpp:
     18        (WebCore::validatePreflightResponse):
     19        * loader/CrossOriginPreflightResultCache.cpp:
     20        (WebCore::CrossOriginPreflightResultCacheItem::create):
     21        (WebCore::CrossOriginPreflightResultCacheItem::validateMethodAndHeaders const):
     22        * loader/CrossOriginPreflightResultCache.h:
     23        (WebCore::CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem):
     24        * platform/network/HTTPParsers.h:
     25        (WebCore::parseAccessControlAllowList):
     26        * platform/network/ResourceResponseBase.cpp:
     27        (WebCore::ResourceResponseBase::filter):
     28        (WebCore::ResourceResponseBase::sanitizeHTTPHeaderFieldsAccordingToTainting):
     29
    1302020-03-18  Joonghun Park  <jh718.park@samsung.com>
    231

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

    r258052 r258631  
    265265        return accessControlCheckResult;
    266266
    267     auto result = makeUnique<CrossOriginPreflightResultCacheItem>(storedCredentialsPolicy);
    268     String errorDescription;
    269     // FIXME: allowsCrossOriginMethod and allowsCrossOriginHeaders should return Expected<void, String> to avoid having an out parameter.
    270     if (!result->parse(response)
    271         || !result->allowsCrossOriginMethod(request.httpMethod(), storedCredentialsPolicy, errorDescription)
    272         || !result->allowsCrossOriginHeaders(request.httpHeaderFields(), storedCredentialsPolicy, errorDescription)) {
    273         return makeUnexpected(errorDescription);
    274     }
    275 
    276     CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), WTFMove(result));
     267    auto result = CrossOriginPreflightResultCacheItem::create(storedCredentialsPolicy, response);
     268    if (!result.has_value())
     269        return makeUnexpected(WTFMove(result.error()));
     270
     271    auto entry = WTFMove(result.value());
     272    auto errorDescription = entry->validateMethodAndHeaders(request.httpMethod(), request.httpHeaderFields());
     273    CrossOriginPreflightResultCache::singleton().appendEntry(securityOrigin.toString(), request.url(), entry.moveToUniquePtr());
     274
     275    if (errorDescription)
     276        return makeUnexpected(WTFMove(*errorDescription));
     277
    277278    return { };
    278279}

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

    r246238 r258631  
    5353}
    5454
    55 bool CrossOriginPreflightResultCacheItem::parse(const ResourceResponse& response)
     55Expected<UniqueRef<CrossOriginPreflightResultCacheItem>, String> CrossOriginPreflightResultCacheItem::create(StoredCredentialsPolicy policy, const ResourceResponse& response)
    5656{
    57     m_methods = parseAccessControlAllowList(response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods));
    58     m_headers = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders));
     57    auto methods = parseAccessControlAllowList(response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods));
     58    if (!methods)
     59        return makeUnexpected(makeString("Header Access-Control-Allow-Methods has an invalid value: ", response.httpHeaderField(HTTPHeaderName::AccessControlAllowMethods)));
     60
     61    auto headers = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders));
     62    if (!headers)
     63        return makeUnexpected(makeString("Header Access-Control-Allow-Headers has an invalid value: ", response.httpHeaderField(HTTPHeaderName::AccessControlAllowHeaders)));
    5964
    6065    Seconds expiryDelta = 0_s;
    … …  
    6570        expiryDelta = defaultPreflightCacheTimeout;
    6671
    67     m_absoluteExpiryTime = MonotonicTime::now() + expiryDelta;
    68     return true;
     72    return makeUniqueRef<CrossOriginPreflightResultCacheItem>(MonotonicTime::now() + expiryDelta, policy, WTFMove(*methods), WTFMove(*headers));
     73}
     74
     75Optional<String> CrossOriginPreflightResultCacheItem::validateMethodAndHeaders(const String& method, const HTTPHeaderMap& requestHeaders) const
     76{
     77    String errorDescription;
     78    if (!allowsCrossOriginMethod(method, m_storedCredentialsPolicy, errorDescription))
     79        return WTFMove(errorDescription);
     80    if (!allowsCrossOriginHeaders(requestHeaders, m_storedCredentialsPolicy, errorDescription))
     81        return WTFMove(errorDescription);
     82    return { };
    6983}
    7084

  • trunk/Source/WebCore/loader/CrossOriginPreflightResultCache.h

    r246238 r258631  
    2828
    2929#include "StoredCredentialsPolicy.h"
     30#include <wtf/Expected.h>
    3031#include <wtf/HashMap.h>
    3132#include <wtf/HashSet.h>
    3233#include <wtf/MonotonicTime.h>
    3334#include <wtf/URLHash.h>
     35#include <wtf/UniqueRef.h>
    3436
    3537namespace WebCore {
    … …  
    4143    WTF_MAKE_NONCOPYABLE(CrossOriginPreflightResultCacheItem); WTF_MAKE_FAST_ALLOCATED;
    4244public:
    43     explicit CrossOriginPreflightResultCacheItem(StoredCredentialsPolicy storedCredentialsPolicy)
    44         : m_storedCredentialsPolicy(storedCredentialsPolicy)
    45     {
    46     }
     45    static Expected<UniqueRef<CrossOriginPreflightResultCacheItem>, String> create(StoredCredentialsPolicy, const ResourceResponse&);
    4746
    48     WEBCORE_EXPORT bool parse(const ResourceResponse&);
    49     WEBCORE_EXPORT bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const;
    50     WEBCORE_EXPORT bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const;
    51     bool allowsRequest(StoredCredentialsPolicy, const String& method, const HTTPHeaderMap& requestHeaders) const;
     47    CrossOriginPreflightResultCacheItem(MonotonicTime, StoredCredentialsPolicy, HashSet<String>&&, HashSet<String, ASCIICaseInsensitiveHash>&&);
     48
     49    Optional<String> validateMethodAndHeaders(const String& method, const HTTPHeaderMap&) const;
     50    bool allowsRequest(StoredCredentialsPolicy, const String& method, const HTTPHeaderMap&) const;
    5251
    5352private:
     53    bool allowsCrossOriginMethod(const String&, StoredCredentialsPolicy, String& errorDescription) const;
     54    bool allowsCrossOriginHeaders(const HTTPHeaderMap&, StoredCredentialsPolicy, String& errorDescription) const;
     55
    5456    // FIXME: A better solution to holding onto the absolute expiration time might be
    5557    // to start a timer for the expiration delta that removes this from the cache when
    … …  
    7678};
    7779
     80inline CrossOriginPreflightResultCacheItem::CrossOriginPreflightResultCacheItem(MonotonicTime absoluteExpiryTime, StoredCredentialsPolicy  storedCredentialsPolicy, HashSet<String>&& methods, HashSet<String, ASCIICaseInsensitiveHash>&& headers)
     81    : m_absoluteExpiryTime(absoluteExpiryTime)
     82    , m_storedCredentialsPolicy(storedCredentialsPolicy)
     83    , m_methods(WTFMove(methods))
     84    , m_headers(WTFMove(headers))
     85{
     86}
     87
    7888} // namespace WebCore

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

    r256013 r258631  
    157157
    158158template<class HashType = DefaultHash<String>::Hash>
    159 HashSet<String, HashType> parseAccessControlAllowList(const String& string)
     159Optional<HashSet<String, HashType>> parseAccessControlAllowList(const String& string)
    160160{
    161161    HashSet<String, HashType> set;
    … …  
    173173            return { };
    174174    }
    175     return set;
     175    return WTFMove(set);
    176176}
    177177

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

    r258458 r258631  
    181181    filteredResponse.setType(Type::Cors);
    182182
    183     auto accessControlExposeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders));
     183    auto accessControlExposeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(response.httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders)).valueOr(HashSet<String, ASCIICaseInsensitiveHash> { });
    184184    if (performCheck == PerformExposeAllHeadersCheck::Yes && accessControlExposeHeaderSet.contains("*"))
    185185        return filteredResponse;
    … …  
    447447        return;
    448448    case ResourceResponse::Tainting::Cors: {
    449         auto corsSafeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders));
     449        auto corsSafeHeaderSet = parseAccessControlAllowList<ASCIICaseInsensitiveHash>(httpHeaderField(HTTPHeaderName::AccessControlExposeHeaders)).valueOr(HashSet<String, ASCIICaseInsensitiveHash> { });
    450450        if (corsSafeHeaderSet.contains("*"))
    451451            return;
Note: See TracChangeset for help on using the changeset viewer.