Changeset 95490 in webkit


Ignore:
Timestamp:
Sep 19, 2011 4:02:52 PM (13 years ago)
Author:
bbudge@chromium.org
Message:

Perform HTTP method and header validation in AssociatedURLLoader for
requests coming from untrusted code (eg. Native Client in Chrome). Use
the same code as XMLHttpRequest to reduce code duplication and have
behavior identical to XHR in Javascript. Add an 'untrustedHTTP' option
to WebURLLoaderOptions, which AssociatedURLLoader can use to determine
if it should check the request method and headers.
https://bugs.webkit.org/show_bug.cgi?id=67655

Reviewed by Darin Fisher.

  • public/WebURLLoaderOptions.h:

(WebKit::WebURLLoaderOptions::WebURLLoaderOptions):

  • src/AssociatedURLLoader.cpp:

(WebKit::AssociatedURLLoader::ClientAdapter::setDelayedError):
(WebKit::AssociatedURLLoader::loadAsynchronously):

  • tests/AssociatedURLLoaderTest.cpp:

(WebKit::AssociatedURLLoaderTest::CheckMethodFails):
(WebKit::AssociatedURLLoaderTest::CheckHeaderFails):
(WebKit::AssociatedURLLoaderTest::CheckFails):
(WebKit::TEST_F):

Location:
trunk/Source/WebKit/chromium
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/chromium/ChangeLog

    r95487 r95490  
     12011-09-19  Bill Budge  <bbudge@chromium.org>
     2
     3        Perform HTTP method and header validation in AssociatedURLLoader for
     4        requests coming from untrusted code (eg. Native Client in Chrome). Use
     5        the same code as XMLHttpRequest to reduce code duplication and have
     6        behavior identical to XHR in Javascript. Add an 'untrustedHTTP' option
     7        to WebURLLoaderOptions, which AssociatedURLLoader can use to determine
     8        if it should check the request method and headers.
     9        https://bugs.webkit.org/show_bug.cgi?id=67655
     10
     11        Reviewed by Darin Fisher.
     12
     13        * public/WebURLLoaderOptions.h:
     14        (WebKit::WebURLLoaderOptions::WebURLLoaderOptions):
     15        * src/AssociatedURLLoader.cpp:
     16        (WebKit::AssociatedURLLoader::ClientAdapter::setDelayedError):
     17        (WebKit::AssociatedURLLoader::loadAsynchronously):
     18        * tests/AssociatedURLLoaderTest.cpp:
     19        (WebKit::AssociatedURLLoaderTest::CheckMethodFails):
     20        (WebKit::AssociatedURLLoaderTest::CheckHeaderFails):
     21        (WebKit::AssociatedURLLoaderTest::CheckFails):
     22        (WebKit::TEST_F):
     23
    1242011-09-19  Adam Barth  <abarth@webkit.org>
    225
  • trunk/Source/WebKit/chromium/public/WebURLLoaderOptions.h

    r83829 r95490  
    4242    };
    4343
    44     WebURLLoaderOptions() : sniffContent(false), allowCredentials(false), forcePreflight(false), crossOriginRequestPolicy(CrossOriginRequestPolicyDeny) { }
     44    WebURLLoaderOptions()
     45        : untrustedHTTP(false)
     46        , sniffContent(false)
     47        , allowCredentials(false)
     48        , forcePreflight(false)
     49        , crossOriginRequestPolicy(CrossOriginRequestPolicyDeny)
     50        { }
    4551
     52    bool untrustedHTTP; // Whether to validate the method and headers as if this was an XMLHttpRequest.
    4653    bool sniffContent; // Whether to sniff content.
    4754    bool allowCredentials; // Whether to send HTTP credentials and cookies with the request.
  • trunk/Source/WebKit/chromium/src/AssociatedURLLoader.cpp

    r94466 r95490  
    3434#include "DocumentThreadableLoader.h"
    3535#include "DocumentThreadableLoaderClient.h"
     36#include "HTTPValidation.h"
    3637#include "SubresourceLoader.h"
    3738#include "Timer.h"
     
    3940#include "WebDataSource.h"
    4041#include "WebFrameImpl.h"
     42#include "WebHTTPHeaderVisitor.h"
    4143#include "WebKit.h"
    4244#include "WebKitPlatformSupport.h"
     
    4648#include "WrappedResourceRequest.h"
    4749#include "WrappedResourceResponse.h"
     50#include "XMLHttpRequest.h"
    4851
    4952using namespace WebCore;
    50 using namespace WebKit;
    5153using namespace WTF;
    5254
    5355namespace WebKit {
     56
     57namespace {
     58
     59class SafeHTTPHeaderValidator : public WebHTTPHeaderVisitor {
     60    WTF_MAKE_NONCOPYABLE(SafeHTTPHeaderValidator);
     61public:
     62    SafeHTTPHeaderValidator() : m_isSafe(true) { }
     63
     64    void visitHeader(const WebString& name, const WebString& value);
     65    bool isSafe() const { return m_isSafe; }
     66
     67private:
     68    bool m_isSafe;
     69};
     70
     71void SafeHTTPHeaderValidator::visitHeader(const WebString& name, const WebString& value)
     72{
     73    m_isSafe = m_isSafe && isValidHTTPToken(name) && XMLHttpRequest::isAllowedHTTPHeader(name) && isValidHTTPHeaderValue(value);
     74}
     75
     76}
    5477
    5578// This class bridges the interface differences between WebCore and WebKit loader clients.
     
    7194
    7295    virtual bool isDocumentThreadableLoaderClient() { return true; }
     96
     97    // Sets an error to be reported back to the client, asychronously.
     98    void setDelayedError(const ResourceError&);
    7399
    74100    // Enables forwarding of error notifications to the WebURLLoaderClient. These must be
     
    178204}
    179205
     206void AssociatedURLLoader::ClientAdapter::setDelayedError(const ResourceError& error)
     207{
     208    didFail(error);
     209}
     210
    180211void AssociatedURLLoader::ClientAdapter::enableErrorNotifications()
    181212{
     
    226257    ASSERT(m_client);
    227258
    228     ThreadableLoaderOptions options;
    229     options.sendLoadCallbacks = SendCallbacks; // Always send callbacks.
    230     options.sniffContent = m_options.sniffContent ? SniffContent : DoNotSniffContent;
    231     options.allowCredentials = m_options.allowCredentials ? AllowStoredCredentials : DoNotAllowStoredCredentials;
    232     options.preflightPolicy = m_options.forcePreflight ? ForcePreflight : ConsiderPreflight;
    233     options.crossOriginRequestPolicy = static_cast<WebCore::CrossOriginRequestPolicy>(m_options.crossOriginRequestPolicy);
    234     options.shouldBufferData = DoNotBufferData;
    235 
    236     const ResourceRequest& webcoreRequest = request.toResourceRequest();
    237     Document* webcoreDocument = m_frameImpl->frame()->document();
     259    bool allowLoad = true;
     260    WebURLRequest newRequest(request);
     261    if (m_options.untrustedHTTP) {
     262        WebString method = newRequest.httpMethod();
     263        allowLoad = isValidHTTPToken(method) && XMLHttpRequest::isAllowedHTTPMethod(method);
     264        if (allowLoad) {
     265            newRequest.setHTTPMethod(XMLHttpRequest::uppercaseKnownHTTPMethod(method));
     266            SafeHTTPHeaderValidator validator;
     267            newRequest.visitHTTPHeaderFields(&validator);
     268            allowLoad = validator.isSafe();
     269        }
     270    }
     271
    238272    m_clientAdapter = ClientAdapter::create(this, m_client, request.downloadToFile());
    239     m_loader = DocumentThreadableLoader::create(webcoreDocument, m_clientAdapter.get(), webcoreRequest, options);
     273
     274    if (allowLoad) {
     275        ThreadableLoaderOptions options;
     276        options.sendLoadCallbacks = SendCallbacks; // Always send callbacks.
     277        options.sniffContent = m_options.sniffContent ? SniffContent : DoNotSniffContent;
     278        options.allowCredentials = m_options.allowCredentials ? AllowStoredCredentials : DoNotAllowStoredCredentials;
     279        options.preflightPolicy = m_options.forcePreflight ? ForcePreflight : ConsiderPreflight;
     280        options.crossOriginRequestPolicy = static_cast<WebCore::CrossOriginRequestPolicy>(m_options.crossOriginRequestPolicy);
     281        options.shouldBufferData = DoNotBufferData;
     282
     283        const ResourceRequest& webcoreRequest = newRequest.toResourceRequest();
     284        Document* webcoreDocument = m_frameImpl->frame()->document();
     285        m_loader = DocumentThreadableLoader::create(webcoreDocument, m_clientAdapter.get(), webcoreRequest, options);
     286    } else {
     287        // FIXME: return meaningful error codes.
     288        m_clientAdapter->setDelayedError(ResourceError());
     289    }
    240290    m_clientAdapter->enableErrorNotifications();
    241291}
  • trunk/Source/WebKit/chromium/tests/AssociatedURLLoaderTest.cpp

    r89089 r95490  
    175175    }
    176176
     177    void CheckMethodFails(const char* unsafeMethod)
     178    {
     179        WebURLRequest request;
     180        request.initialize();
     181        request.setURL(GURL("http://www.test.com/success.html"));
     182        request.setHTTPMethod(WebString::fromUTF8(unsafeMethod));
     183        WebURLLoaderOptions options;
     184        options.untrustedHTTP = true;
     185        CheckFails(request, options);
     186    }
     187
     188    void CheckHeaderFails(const char* headerField)
     189    {
     190        CheckHeaderFails(headerField, "foo");
     191    }
     192
     193    void CheckHeaderFails(const char* headerField, const char* headerValue)
     194    {
     195        WebURLRequest request;
     196        request.initialize();
     197        request.setURL(GURL("http://www.test.com/success.html"));
     198        request.setHTTPHeaderField(WebString::fromUTF8(headerField), WebString::fromUTF8(headerValue));
     199        WebURLLoaderOptions options;
     200        options.untrustedHTTP = true;
     201        CheckFails(request, options);
     202    }
     203
     204    void CheckFails(const WebURLRequest& request, WebURLLoaderOptions options = WebURLLoaderOptions())
     205    {
     206        m_expectedLoader = createAssociatedURLLoader(options);
     207        EXPECT_TRUE(m_expectedLoader);
     208        m_didFail = false;
     209        m_expectedLoader->loadAsynchronously(request, this);
     210        // Failure should not be reported synchronously.
     211        EXPECT_FALSE(m_didFail);
     212        // Allow the loader to return the error.
     213        webkit_support::RunMessageLoop();
     214        EXPECT_TRUE(m_didFail);
     215    }
     216
    177217protected:
    178218    WebString m_frameFilePath;
     
    224264    request.initialize();
    225265    request.setURL(url);
    226 
    227     m_expectedLoader = createAssociatedURLLoader();
    228     EXPECT_TRUE(m_expectedLoader);
    229     m_expectedLoader->loadAsynchronously(request, this);
    230     // Failure should not be reported synchronously.
    231     EXPECT_FALSE(m_didFail);
    232     // Allow the loader to return the error.
    233     webkit_support::RunMessageLoop();
    234     EXPECT_TRUE(m_didFail);
     266    CheckFails(request);
    235267}
    236268
     
    260292}
    261293
    262 }
     294// Test that untrusted loads can't use a forbidden method.
     295TEST_F(AssociatedURLLoaderTest, UntrustedCheckMethods)
     296{
     297    // Check non-token method fails.
     298    CheckMethodFails("GET()");
     299    CheckMethodFails("POST\x0d\x0ax-csrf-token:\x20test1234");
     300
     301    // Forbidden methods should fail regardless of casing.
     302    CheckMethodFails("CoNneCt");
     303    CheckMethodFails("TrAcK");
     304    CheckMethodFails("TrAcE");
     305}
     306
     307// Test that untrusted loads can't use a forbidden header field.
     308TEST_F(AssociatedURLLoaderTest, UntrustedCheckHeaders)
     309{
     310    // Check non-token header fails.
     311    CheckHeaderFails("foo()");
     312
     313    // Check forbidden headers fail.
     314    CheckHeaderFails("accept-charset");
     315    CheckHeaderFails("accept-encoding");
     316    CheckHeaderFails("connection");
     317    CheckHeaderFails("content-length");
     318    CheckHeaderFails("cookie");
     319    CheckHeaderFails("cookie2");
     320    CheckHeaderFails("content-transfer-encoding");
     321    CheckHeaderFails("date");
     322    CheckHeaderFails("expect");
     323    CheckHeaderFails("host");
     324    CheckHeaderFails("keep-alive");
     325    CheckHeaderFails("origin");
     326    CheckHeaderFails("referer");
     327    CheckHeaderFails("te");
     328    CheckHeaderFails("trailer");
     329    CheckHeaderFails("transfer-encoding");
     330    CheckHeaderFails("upgrade");
     331    CheckHeaderFails("user-agent");
     332    CheckHeaderFails("via");
     333
     334    CheckHeaderFails("proxy-");
     335    CheckHeaderFails("proxy-foo");
     336    CheckHeaderFails("sec-");
     337    CheckHeaderFails("sec-foo");
     338
     339    // Check that validation is case-insensitive.
     340    CheckHeaderFails("AcCePt-ChArSeT");
     341    CheckHeaderFails("ProXy-FoO");
     342
     343    // Check invalid header values.
     344    CheckHeaderFails("foo", "bar\x0d\x0ax-csrf-token:\x20test1234");
     345}
     346
     347}
Note: See TracChangeset for help on using the changeset viewer.