Changeset 206716 in webkit


Ignore:
Timestamp:
Oct 2, 2016 6:59:18 AM (7 years ago)
Author:
commit-queue@webkit.org
Message:

[Fetch API] Forbid redirection to non-HTTP(s) URL in non-navigation mode.
https://bugs.webkit.org/show_bug.cgi?id=162785

Patch by Youenn Fablet <youenn@apple.com> on 2016-10-02
Reviewed by Alex Christensen.

LayoutTests/imported/w3c:

  • web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt:
  • web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt:

Source/WebCore:

Covered by rebased and existing tests.

Ensuring non-HTTP redirection URLs are not followed at DocumentThreadableLoader level for fetch API only.
This should be applied to all clients at some point, but there is still some uncertainty for data URLs.

Did some refactoring to better separate the case of security checks in case of regular request or redirected request.
This allows in particular to handle more clearly the case of data URLs which are allowed in all modes for regular requests.
But they are not allowed for same-origin redirected requests.

  • WebCore.xcodeproj/project.pbxproj:
  • loader/DocumentThreadableLoader.cpp:

(WebCore::reportRedirectionWithBadScheme): Reporting bad scheme redirection error.
(WebCore::DocumentThreadableLoader::redirectReceived): Checking that redirection URLs are HTTP(s) in case of Fetch API.

  • loader/SubresourceLoader.cpp:

(WebCore::SubresourceLoader::willSendRequestInternal):

  • loader/cache/CachedResourceLoader.cpp:

(WebCore::CachedResourceLoader::requestImage):
(WebCore::CachedResourceLoader::checkInsecureContent):
(WebCore::CachedResourceLoader::allowedByContentSecurityPolicy):
(WebCore::isSameOriginDataURL):
(WebCore::CachedResourceLoader::canRequest):
(WebCore::CachedResourceLoader::canRequestAfterRedirection):
(WebCore::CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox):
(WebCore::CachedResourceLoader::requestResource):

  • loader/cache/CachedResourceLoader.h:
Location:
trunk
Files:
9 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/imported/w3c/ChangeLog

    r206663 r206716  
     12016-10-02  Youenn Fablet  <youenn@apple.com>
     2
     3        [Fetch API] Forbid redirection to non-HTTP(s) URL in non-navigation mode.
     4        https://bugs.webkit.org/show_bug.cgi?id=162785
     5
     6        Reviewed by Alex Christensen.
     7
     8        * web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt:
     9        * web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt:
     10
    1112016-09-30  Chris Dumez  <cdumez@apple.com>
    212
  • trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-expected.txt

    r206260 r206716  
    55
    66PASS Testing data URL loading after same-origin redirection (cors mode)
    7 FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
     7PASS Testing data URL loading after same-origin redirection (no-cors mode)
    88PASS Testing data URL loading after same-origin redirection (same-origin mode)
    99PASS Testing data URL loading after cross-origin redirection (cors mode)
    10 FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
     10PASS Testing data URL loading after cross-origin redirection (no-cors mode)
    1111
  • trunk/LayoutTests/imported/w3c/web-platform-tests/fetch/api/redirect/redirect-to-dataurl-worker-expected.txt

    r206260 r206716  
    55
    66PASS Testing data URL loading after same-origin redirection (cors mode)
    7 FAIL Testing data URL loading after same-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
     7PASS Testing data URL loading after same-origin redirection (no-cors mode)
    88PASS Testing data URL loading after same-origin redirection (same-origin mode)
    99PASS Testing data URL loading after cross-origin redirection (cors mode)
    10 FAIL Testing data URL loading after cross-origin redirection (no-cors mode) assert_unreached: Should have rejected. Reached unreachable code
     10PASS Testing data URL loading after cross-origin redirection (no-cors mode)
    1111
  • trunk/Source/WebCore/ChangeLog

    r206713 r206716  
     12016-10-02  Youenn Fablet  <youenn@apple.com>
     2
     3        [Fetch API] Forbid redirection to non-HTTP(s) URL in non-navigation mode.
     4        https://bugs.webkit.org/show_bug.cgi?id=162785
     5
     6        Reviewed by Alex Christensen.
     7
     8        Covered by rebased and existing tests.
     9
     10        Ensuring non-HTTP redirection URLs are not followed at DocumentThreadableLoader level for fetch API only.
     11        This should be applied to all clients at some point, but there is still some uncertainty for data URLs.
     12
     13        Did some refactoring to better separate the case of security checks in case of regular request or redirected request.
     14        This allows in particular to handle more clearly the case of data URLs which are allowed in all modes for regular requests.
     15        But they are not allowed for same-origin redirected requests.
     16
     17        * WebCore.xcodeproj/project.pbxproj:
     18        * loader/DocumentThreadableLoader.cpp:
     19        (WebCore::reportRedirectionWithBadScheme): Reporting bad scheme redirection error.
     20        (WebCore::DocumentThreadableLoader::redirectReceived): Checking that redirection URLs are HTTP(s) in case of Fetch API.
     21        * loader/SubresourceLoader.cpp:
     22        (WebCore::SubresourceLoader::willSendRequestInternal):
     23        * loader/cache/CachedResourceLoader.cpp:
     24        (WebCore::CachedResourceLoader::requestImage):
     25        (WebCore::CachedResourceLoader::checkInsecureContent):
     26        (WebCore::CachedResourceLoader::allowedByContentSecurityPolicy):
     27        (WebCore::isSameOriginDataURL):
     28        (WebCore::CachedResourceLoader::canRequest):
     29        (WebCore::CachedResourceLoader::canRequestAfterRedirection):
     30        (WebCore::CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox):
     31        (WebCore::CachedResourceLoader::requestResource):
     32        * loader/cache/CachedResourceLoader.h:
     33
    1342016-10-01  Simon Fraser  <simon.fraser@apple.com>
    235
  • trunk/Source/WebCore/WebCore.xcodeproj/project.pbxproj

    r206691 r206716  
    57925792                CE2849891CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE2849881CA3614600B4A57F /* ContentSecurityPolicyDirectiveNames.cpp */; };
    57935793                CE6DADF91C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE6DADF71C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.cpp */; };
    5794                 CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; };
     5794                CE6DADFA1C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h in Headers */ = {isa = PBXBuildFile; fileRef = CE6DADF81C591E6A003F6A88 /* ContentSecurityPolicyResponseHeaders.h */; settings = {ATTRIBUTES = (Private, ); }; };
    57955795                CE799F971C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp in Sources */ = {isa = PBXBuildFile; fileRef = CE799F951C6A46BC0097B518 /* ContentSecurityPolicySourceList.cpp */; };
    57965796                CE799F981C6A46BC0097B518 /* ContentSecurityPolicySourceList.h in Headers */ = {isa = PBXBuildFile; fileRef = CE799F961C6A46BC0097B518 /* ContentSecurityPolicySourceList.h */; };
  • trunk/Source/WebCore/loader/DocumentThreadableLoader.cpp

    r206524 r206716  
    217217}
    218218
     219static inline void reportRedirectionWithBadScheme(ThreadableLoaderClient& client, const URL& url)
     220{
     221    client.didFail(ResourceError(errorDomainWebKitInternal, 0, url, "Redirection to URL with a scheme that is not HTTP(S).", ResourceError::Type::AccessControl));
     222}
     223
    219224void DocumentThreadableLoader::redirectReceived(CachedResource* resource, ResourceRequest& request, const ResourceResponse& redirectResponse)
    220225{
     
    223228
    224229    Ref<DocumentThreadableLoader> protectedThis(*this);
     230
     231    // FIXME: We restrict this check to Fetch API for the moment, as this might disrupt WorkerScriptLoader.
     232    // Reassess this check based on https://github.com/whatwg/fetch/issues/393 discussions.
     233    // We should also disable that check in navigation mode.
     234    if (!request.url().protocolIsInHTTPFamily() && m_options.initiator == cachedResourceRequestInitiators().fetch) {
     235        reportRedirectionWithBadScheme(*m_client, request.url());
     236        clearResource();
     237        return;
     238    }
     239
    225240    if (!isAllowedByContentSecurityPolicy(request.url(), redirectResponse.isNull() ? ContentSecurityPolicy::RedirectResponseReceived::No : ContentSecurityPolicy::RedirectResponseReceived::Yes)) {
    226241        reportContentSecurityPolicyError(*m_client, redirectResponse.url());
  • trunk/Source/WebCore/loader/SubresourceLoader.cpp

    r205481 r206716  
    201201        }
    202202
    203         if (!m_documentLoader->cachedResourceLoader().canRequest(m_resource->type(), newRequest.url(), options(), false /* forPreload */, true /* didReceiveRedirectResponse */)) {
     203        if (!m_documentLoader->cachedResourceLoader().canRequestAfterRedirection(m_resource->type(), newRequest.url(), options())) {
    204204            cancel();
    205205            return;
  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.cpp

    r206255 r206716  
    184184                document->contentSecurityPolicy()->upgradeInsecureRequestIfNeeded(request.mutableResourceRequest(), ContentSecurityPolicy::InsecureRequestType::Load);
    185185            URL requestURL = request.resourceRequest().url();
    186             if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request.options(), request.forPreload()))
     186            if (requestURL.isValid() && canRequest(CachedResource::ImageResource, requestURL, request))
    187187                PingLoader::loadImage(*frame, requestURL);
    188188            return nullptr;
     
    334334bool CachedResourceLoader::checkInsecureContent(CachedResource::Type type, const URL& url) const
    335335{
     336
     337    if (!canRequestInContentDispositionAttachmentSandbox(type, url))
     338        return false;
     339
    336340    switch (type) {
    337341    case CachedResource::Script:
     
    380384}
    381385
    382 static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
    383 {
    384     return !didReceiveRedirectResponse && url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
    385 }
    386 
    387 bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool didReceiveRedirectResponse)
     386bool CachedResourceLoader::allowedByContentSecurityPolicy(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, ContentSecurityPolicy::RedirectResponseReceived redirectResponseReceived)
    388387{
    389388    if (options.contentSecurityPolicyImposition == ContentSecurityPolicyImposition::SkipPolicyCheck)
     
    392391    ASSERT(m_document);
    393392    ASSERT(m_document->contentSecurityPolicy());
    394 
    395     auto redirectResponseReceived = didReceiveRedirectResponse ? ContentSecurityPolicy::RedirectResponseReceived::Yes : ContentSecurityPolicy::RedirectResponseReceived::No;
    396393
    397394    switch (type) {
     
    431428        ASSERT_NOT_REACHED();
    432429    }
     430
    433431    return true;
    434432}
    435433
    436 bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options, bool forPreload, bool didReceiveRedirectResponse)
    437 {
     434static inline bool isSameOriginDataURL(const URL& url, const ResourceLoaderOptions& options)
     435{
     436    // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (https://github.com/whatwg/fetch/issues/381).
     437    return url.protocolIsData() && options.sameOriginDataURLFlag == SameOriginDataURLFlag::Set;
     438}
     439
     440bool CachedResourceLoader::canRequest(CachedResource::Type type, const URL& url, const CachedResourceRequest& request)
     441{
     442    auto& options = request.options();
     443
    438444    if (document() && !document()->securityOrigin()->canDisplay(url)) {
    439         if (!forPreload)
     445        if (!request.forPreload())
    440446            FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
    441447        LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
     
    443449    }
    444450
    445     // FIXME: Remove same-origin data URL flag since it was removed from fetch spec (see https://github.com/whatwg/fetch/issues/381).
    446     if (options.mode == FetchOptions::Mode::SameOrigin && !isSameOriginDataURL(url, options, didReceiveRedirectResponse) && !m_document->securityOrigin()->canRequest(url)) {
     451    if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url) && !isSameOriginDataURL(url, options)) {
    447452        printAccessDeniedMessage(url);
    448453        return false;
    449454    }
    450455
    451     if (!allowedByContentSecurityPolicy(type, url, options, didReceiveRedirectResponse))
     456    if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::No))
    452457        return false;
    453458
     
    458463    }
    459464
    460     if (!canRequestInContentDispositionAttachmentSandbox(type, url))
    461         return false;
    462 
    463     // Last of all, check for insecure content. We do this last so that when
    464     // folks block insecure content with a CSP policy, they don't get a warning.
     465    // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
    465466    // They'll still get a warning in the console about CSP blocking the load.
    466467
    467     // FIXME: Should we consider forPreload here?
     468    // FIXME: Should we consider whether the request is for preload here?
    468469    if (!checkInsecureContent(type, url))
    469470        return false;
     
    472473}
    473474
     475// FIXME: Should we find a way to know whether the redirection is for a preload request like we do for CachedResourceLoader::canRequest?
     476bool CachedResourceLoader::canRequestAfterRedirection(CachedResource::Type type, const URL& url, const ResourceLoaderOptions& options)
     477{
     478    if (document() && !document()->securityOrigin()->canDisplay(url)) {
     479        FrameLoader::reportLocalLoadFailed(frame(), url.stringCenterEllipsizedToLength());
     480        LOG(ResourceLoading, "CachedResourceLoader::requestResource URL was not allowed by SecurityOrigin::canDisplay");
     481        return false;
     482    }
     483
     484    // FIXME: According to https://fetch.spec.whatwg.org/#http-redirect-fetch, we should check that the URL is HTTP(s) except if in navigation mode.
     485    // But we currently allow at least data URLs to be loaded.
     486
     487    if (options.mode == FetchOptions::Mode::SameOrigin && !m_document->securityOrigin()->canRequest(url)) {
     488        printAccessDeniedMessage(url);
     489        return false;
     490    }
     491
     492    if (!allowedByContentSecurityPolicy(type, url, options, ContentSecurityPolicy::RedirectResponseReceived::Yes))
     493        return false;
     494
     495    // Last of all, check for insecure content. We do this last so that when folks block insecure content with a CSP policy, they don't get a warning.
     496    // They'll still get a warning in the console about CSP blocking the load.
     497    if (!checkInsecureContent(type, url))
     498        return false;
     499
     500    return true;
     501}
     502
    474503bool CachedResourceLoader::canRequestInContentDispositionAttachmentSandbox(CachedResource::Type type, const URL& url) const
    475504{
    476505    Document* document;
    477    
     506
    478507    // FIXME: Do we want to expand this to all resource types that the mixed content checker would consider active content?
    479508    switch (type) {
     
    625654    prepareFetch(type, request);
    626655
    627     if (!canRequest(type, url, request.options(), request.forPreload())) {
     656    // We are passing url as well as request, as request url may contain a fragment identifier.
     657    if (!canRequest(type, url, request)) {
    628658        RELEASE_LOG_IF_ALLOWED("requestResource: Not allowed to request resource (frame = %p)", frame());
    629659        return nullptr;
  • trunk/Source/WebCore/loader/cache/CachedResourceLoader.h

    r206206 r206716  
    3131#include "CachedResourceHandle.h"
    3232#include "CachedResourceRequest.h"
     33#include "ContentSecurityPolicy.h"
    3334#include "ResourceLoadPriority.h"
    3435#include "ResourceTimingInformation.h"
     
    123124
    124125    WEBCORE_EXPORT void garbageCollectDocumentResources();
    125    
     126
    126127    void incrementRequestCount(const CachedResource&);
    127128    void decrementRequestCount(const CachedResource&);
     
    136137    void printPreloadStats();
    137138
    138     bool canRequest(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool forPreload = false, bool didReceiveRedirectResponse = false);
     139    bool canRequest(CachedResource::Type, const URL&, const CachedResourceRequest&);
     140    bool canRequestAfterRedirection(CachedResource::Type, const URL&, const ResourceLoaderOptions&);
    139141
    140142    static const ResourceLoaderOptions& defaultCachedResourceOptions();
     
    165167    bool shouldContinueAfterNotifyingLoadedFromMemoryCache(const CachedResourceRequest&, CachedResource*);
    166168    bool checkInsecureContent(CachedResource::Type, const URL&) const;
    167     bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, bool);
     169    bool allowedByContentSecurityPolicy(CachedResource::Type, const URL&, const ResourceLoaderOptions&, ContentSecurityPolicy::RedirectResponseReceived);
    168170
    169171    void performPostLoadActions();
Note: See TracChangeset for help on using the changeset viewer.