Changeset 142099 in webkit


Ignore:
Timestamp:
Feb 7, 2013 5:01:02 AM (11 years ago)
Author:
tonyg@chromium.org
Message:

Call XSSAuditor.filterToken() from threaded HTML parser
​https://bugs.webkit.org/show_bug.cgi?id=107603

Reviewed by Adam Barth.

With this patch we now pass 180 of 182 tests in http/tests/security/xssAuditor.

We do this by creating aan XSSAuditor on the main thread and passing ownership of them to the BackgroundHTMLParser upon its creation.

Then the background thread calls filterToken() and stores the resulting XSSInfo (if any) on the CompactHTMLToken for the main thread to handle.

This involved trimming the XSSAuditor to only depend on the TextEncoding instead of the whole TextResourceDecoder.

No new tests because covered by existing tests.

  • html/parser/BackgroundHTMLParser.cpp:

(WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
(WebCore::BackgroundHTMLParser::pumpTokenizer):
(WebCore::BackgroundHTMLParser::createPartial):

  • html/parser/BackgroundHTMLParser.h:

(WebCore):
(WebCore::BackgroundHTMLParser::create):
(BackgroundHTMLParser):

  • html/parser/HTMLDocumentParser.cpp:

(WebCore::HTMLDocumentParser::pumpTokenizer):
(WebCore::HTMLDocumentParser::startBackgroundParser):

  • html/parser/HTMLSourceTracker.cpp:

(WebCore::HTMLSourceTracker::start):
(WebCore::HTMLSourceTracker::end):

  • html/parser/HTMLSourceTracker.h: Change the HTMLInputStream args to SegmentedString because the background thread only has a BackgroundHTMLInputStream.

(HTMLSourceTracker):

  • html/parser/HTMLViewSourceParser.cpp:

(WebCore::HTMLViewSourceParser::pumpTokenizer):

  • html/parser/XSSAuditor.cpp:

(WebCore::fullyDecodeString):
(WebCore::XSSAuditor::XSSAuditor):
(WebCore::XSSAuditor::init): Copies necessary to make isSafeToSendToAnotherThread() happy.
(WebCore::XSSAuditor::decodedSnippetForName):
(WebCore::XSSAuditor::decodedSnippetForAttribute):
(WebCore::XSSAuditor::decodedSnippetForJavaScript):
(WebCore::XSSAuditor::isSafeToSendToAnotherThread): Check that all String and KURL members are safe to send to another thread.
(WebCore):

  • html/parser/XSSAuditor.h:

(WebCore):
(WebCore::FilterTokenRequest::FilterTokenRequest):
(FilterTokenRequest):
(XSSAuditor):

Location:
trunk/Source/WebCore
Files:
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r142094 r142099  
     12013-02-07  Tony Gentilcore  <tonyg@chromium.org>
     2
     3        Call XSSAuditor.filterToken() from threaded HTML parser
     4        https://bugs.webkit.org/show_bug.cgi?id=107603
     5
     6        Reviewed by Adam Barth.
     7
     8        With this patch we now pass 180 of 182 tests in http/tests/security/xssAuditor.
     9
     10        We do this by creating aan XSSAuditor on the main thread and passing ownership of them to the BackgroundHTMLParser upon its creation.
     11
     12        Then the background thread calls filterToken() and stores the resulting XSSInfo (if any) on the CompactHTMLToken for the main thread to handle.
     13
     14        This involved trimming the XSSAuditor to only depend on the TextEncoding instead of the whole TextResourceDecoder.
     15
     16        No new tests because covered by existing tests.
     17
     18        * html/parser/BackgroundHTMLParser.cpp:
     19        (WebCore::BackgroundHTMLParser::BackgroundHTMLParser):
     20        (WebCore::BackgroundHTMLParser::pumpTokenizer):
     21        (WebCore::BackgroundHTMLParser::createPartial):
     22        * html/parser/BackgroundHTMLParser.h:
     23        (WebCore):
     24        (WebCore::BackgroundHTMLParser::create):
     25        (BackgroundHTMLParser):
     26        * html/parser/HTMLDocumentParser.cpp:
     27        (WebCore::HTMLDocumentParser::pumpTokenizer):
     28        (WebCore::HTMLDocumentParser::startBackgroundParser):
     29        * html/parser/HTMLSourceTracker.cpp:
     30        (WebCore::HTMLSourceTracker::start):
     31        (WebCore::HTMLSourceTracker::end):
     32        * html/parser/HTMLSourceTracker.h: Change the HTMLInputStream args to SegmentedString because the background thread only has a BackgroundHTMLInputStream.
     33        (HTMLSourceTracker):
     34        * html/parser/HTMLViewSourceParser.cpp:
     35        (WebCore::HTMLViewSourceParser::pumpTokenizer):
     36        * html/parser/XSSAuditor.cpp:
     37        (WebCore::fullyDecodeString):
     38        (WebCore::XSSAuditor::XSSAuditor):
     39        (WebCore::XSSAuditor::init): Copies necessary to make isSafeToSendToAnotherThread() happy.
     40        (WebCore::XSSAuditor::decodedSnippetForName):
     41        (WebCore::XSSAuditor::decodedSnippetForAttribute):
     42        (WebCore::XSSAuditor::decodedSnippetForJavaScript):
     43        (WebCore::XSSAuditor::isSafeToSendToAnotherThread): Check that all String and KURL members are safe to send to another thread.
     44        (WebCore):
     45        * html/parser/XSSAuditor.h:
     46        (WebCore):
     47        (WebCore::FilterTokenRequest::FilterTokenRequest):
     48        (FilterTokenRequest):
     49        (XSSAuditor):
     50
    1512013-02-07  ChangSeok Oh  <shivamidow@gmail.com>
    252

  • trunk/Source/WebCore/html/parser/BackgroundHTMLParser.cpp

    r142004 r142099  
    3737#include "MathMLNames.h"
    3838#include "SVGNames.h"
     39#include "XSSAuditor.h"
    3940#include <wtf/MainThread.h>
    4041#include <wtf/Vector.h>
    … …  
    7273}
    7374
    74 BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser)
     75BackgroundHTMLParser::BackgroundHTMLParser(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
    7576    : m_inForeignContent(false)
    7677    , m_token(adoptPtr(new HTMLToken))
    … …  
    7980    , m_parser(parser)
    8081    , m_pendingTokens(adoptPtr(new CompactHTMLTokenStream))
     82    , m_xssAuditor(xssAuditor)
    8183{
    8284}
    … …  
    153155void BackgroundHTMLParser::pumpTokenizer()
    154156{
    155     while (m_tokenizer->nextToken(m_input.current(), *m_token.get())) {
    156         // FIXME: Call m_xssAuditor.filterToken(m_token) and put resulting XSSInfo into CompactHTMLToken.
    157         m_pendingTokens->append(CompactHTMLToken(m_token.get(), TextPosition(m_input.current().currentLine(), m_input.current().currentColumn())));
     157    while (true) {
     158        m_sourceTracker.start(m_input.current(), m_tokenizer.get(), *m_token);
     159        if (!m_tokenizer->nextToken(m_input.current(), *m_token.get()))
     160            break;
     161        m_sourceTracker.end(m_input.current(), m_tokenizer.get(), *m_token);
     162
     163        {
     164            OwnPtr<XSSInfo> xssInfo = m_xssAuditor->filterToken(FilterTokenRequest(*m_token, m_sourceTracker, m_tokenizer->shouldAllowCDATA()));
     165            CompactHTMLToken token(m_token.get(), TextPosition(m_input.current().currentLine(), m_input.current().currentColumn()));
     166            if (xssInfo)
     167                token.setXSSInfo(xssInfo.release());
     168            m_pendingTokens->append(token);
     169        }
     170
    158171        m_token->clear();
    159172
    … …  
    182195}
    183196
    184 void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser)
     197void BackgroundHTMLParser::createPartial(ParserIdentifier identifier, const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
    185198{
    186199    ASSERT(!parserMap().backgroundParsers().get(identifier));
    187     parserMap().backgroundParsers().set(identifier, BackgroundHTMLParser::create(options, parser));
     200    parserMap().backgroundParsers().set(identifier, BackgroundHTMLParser::create(options, parser, xssAuditor));
    188201}
    189202

  • trunk/Source/WebCore/html/parser/BackgroundHTMLParser.h

    r141363 r142099  
    3232#include "CompactHTMLToken.h"
    3333#include "HTMLParserOptions.h"
     34#include "HTMLSourceTracker.h"
    3435#include "HTMLToken.h"
    3536#include "HTMLTokenizer.h"
     37#include <wtf/PassOwnPtr.h>
     38#include <wtf/RefPtr.h>
    3639#include <wtf/WeakPtr.h>
    3740
    … …  
    4043typedef const void* ParserIdentifier;
    4144class HTMLDocumentParser;
     45class XSSAuditor;
    4246
    4347class BackgroundHTMLParser {
    … …  
    4852    void finish();
    4953
    50     static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser)
     54    static PassOwnPtr<BackgroundHTMLParser> create(const HTMLParserOptions& options, const WeakPtr<HTMLDocumentParser>& parser, PassOwnPtr<XSSAuditor> xssAuditor)
    5155    {
    52         return adoptPtr(new BackgroundHTMLParser(options, parser));
     56        return adoptPtr(new BackgroundHTMLParser(options, parser, xssAuditor));
    5357    }
    5458
    55     static void createPartial(ParserIdentifier, const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&);
     59    static void createPartial(ParserIdentifier, const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
    5660    static void stopPartial(ParserIdentifier);
    5761    static void appendPartial(ParserIdentifier, const String& input);
    … …  
    6064
    6165private:
    62     BackgroundHTMLParser(const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&);
     66    BackgroundHTMLParser(const HTMLParserOptions&, const WeakPtr<HTMLDocumentParser>&, PassOwnPtr<XSSAuditor>);
    6367
    6468    void markEndOfFile();
    … …  
    7074    bool m_inForeignContent; // FIXME: We need a stack of foreign content markers.
    7175    BackgroundHTMLInputStream m_input;
     76    HTMLSourceTracker m_sourceTracker;
    7277    OwnPtr<HTMLToken> m_token;
    7378    OwnPtr<HTMLTokenizer> m_tokenizer;
    … …  
    7580    WeakPtr<HTMLDocumentParser> m_parser;
    7681    OwnPtr<CompactHTMLTokenStream> m_pendingTokens;
     82    OwnPtr<XSSAuditor> m_xssAuditor;
    7783};
    7884

  • trunk/Source/WebCore/html/parser/HTMLDocumentParser.cpp

    r142004 r142099  
    3131#include "ContentSecurityPolicy.h"
    3232#include "DocumentFragment.h"
     33#include "DocumentLoader.h"
    3334#include "Element.h"
    3435#include "Frame.h"
    … …  
    369370    while (canTakeNextToken(mode, session) && !session.needsYield) {
    370371        if (!isParsingFragment())
    371             m_sourceTracker.start(m_input, m_tokenizer.get(), token());
     372            m_sourceTracker.start(m_input.current(), m_tokenizer.get(), token());
    372373
    373374        if (!m_tokenizer->nextToken(m_input.current(), token()))
    … …  
    375376
    376377        if (!isParsingFragment()) {
    377             m_sourceTracker.end(m_input, m_tokenizer.get(), token());
     378            m_sourceTracker.end(m_input.current(), m_tokenizer.get(), token());
    378379
    379380            // We do not XSS filter innerHTML, which means we (intentionally) fail
    380381            // http/tests/security/xssAuditor/dom-write-innerHTML.html
    381             if (OwnPtr<XSSInfo> xssInfo = m_xssAuditor.filterToken(FilterTokenRequest(token(), m_sourceTracker, document()->decoder(), m_tokenizer->shouldAllowCDATA())))
     382            if (OwnPtr<XSSInfo> xssInfo = m_xssAuditor.filterToken(FilterTokenRequest(token(), m_sourceTracker, m_tokenizer->shouldAllowCDATA())))
    382383                m_xssAuditorDelegate.didBlockScript(*xssInfo);
    383384        }
    … …  
    512513    m_haveBackgroundParser = true;
    513514
     515    WeakPtr<HTMLDocumentParser> parser = m_weakFactory.createWeakPtr();
     516    OwnPtr<XSSAuditor> xssAuditor = adoptPtr(new XSSAuditor);
     517    xssAuditor->init(document());
     518    ASSERT(xssAuditor->isSafeToSendToAnotherThread());
    514519    ParserIdentifier identifier = ParserMap::identifierForParser(this);
    515     WeakPtr<HTMLDocumentParser> parser = m_weakFactory.createWeakPtr();
    516     HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::createPartial, identifier, m_options, parser));
     520    HTMLParserThread::shared()->postTask(bind(&BackgroundHTMLParser::createPartial, identifier, m_options, parser, xssAuditor.release()));
    517521}
    518522

  • trunk/Source/WebCore/html/parser/HTMLSourceTracker.cpp

    r124679 r142099  
    3535}
    3636
    37 void HTMLSourceTracker::start(const HTMLInputStream& input, HTMLTokenizer* tokenizer, HTMLToken& token)
     37void HTMLSourceTracker::start(SegmentedString& currentInput, HTMLTokenizer* tokenizer, HTMLToken& token)
    3838{
    3939    if (token.type() == HTMLTokenTypes::Uninitialized) {
    … …  
    4444        m_previousSource.append(m_currentSource);
    4545
    46     m_currentSource = input.current();
     46    m_currentSource = currentInput;
    4747    token.setBaseOffset(m_currentSource.numberOfCharactersConsumed() - m_previousSource.length());
    4848}
    4949
    50 void HTMLSourceTracker::end(const HTMLInputStream& input, HTMLTokenizer* tokenizer, HTMLToken& token)
     50void HTMLSourceTracker::end(SegmentedString& currentInput, HTMLTokenizer* tokenizer, HTMLToken& token)
    5151{
    5252    m_cachedSourceForToken = String();
    5353
    5454    // FIXME: This work should really be done by the HTMLTokenizer.
    55     token.end(input.current().numberOfCharactersConsumed() - tokenizer->numberOfBufferedCharacters());
     55    token.end(currentInput.numberOfCharactersConsumed() - tokenizer->numberOfBufferedCharacters());
    5656}
    5757

  • trunk/Source/WebCore/html/parser/HTMLSourceTracker.h

    r103999 r142099  
    2727#define HTMLSourceTracker_h
    2828
    29 #include "HTMLInputStream.h"
    3029#include "HTMLToken.h"
     30#include "SegmentedString.h"
    3131#include <wtf/text/StringBuilder.h>
    3232
    … …  
    4343    // something that makes it obvious that this method can be called multiple
    4444    // times.
    45     void start(const HTMLInputStream&, HTMLTokenizer*, HTMLToken&);
    46     void end(const HTMLInputStream&, HTMLTokenizer*, HTMLToken&);
     45    void start(SegmentedString&, HTMLTokenizer*, HTMLToken&);
     46    void end(SegmentedString&, HTMLTokenizer*, HTMLToken&);
    4747
    4848    String sourceForToken(const HTMLToken&);

  • trunk/Source/WebCore/html/parser/HTMLViewSourceParser.cpp

    r140036 r142099  
    5252{
    5353    while (true) {
    54         m_sourceTracker.start(m_input, m_tokenizer.get(), m_token);
     54        m_sourceTracker.start(m_input.current(), m_tokenizer.get(), m_token);
    5555        if (!m_tokenizer->nextToken(m_input.current(), m_token))
    5656            break;
    57         m_sourceTracker.end(m_input, m_tokenizer.get(), m_token);
     57        m_sourceTracker.end(m_input.current(), m_tokenizer.get(), m_token);
    5858
    5959        document()->addSource(sourceForToken(), m_token);

  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r142004 r142099  
    161161}
    162162
    163 static String fullyDecodeString(const String& string, const TextResourceDecoder* decoder)
    164 {
    165     const TextEncoding& encoding = decoder ? decoder->encoding() : UTF8Encoding();
     163static String fullyDecodeString(const String& string, const TextEncoding& encoding)
     164{
    166165    size_t oldWorkingStringLength;
    167166    String workingString = string;
    … …  
    180179    , m_state(Uninitialized)
    181180    , m_scriptTagNestingLevel(0)
     181    , m_encoding(UTF8Encoding())
    182182{
    183183    // Although tempting to call init() at this point, the various objects
    … …  
    203203        return;
    204204
    205     m_documentURL = document->url();
     205    m_documentURL = document->url().copy();
    206206
    207207    // In theory, the Document could have detached from the Frame after the
    … …  
    223223    }
    224224
    225     TextResourceDecoder* decoder = document->decoder();
    226     m_decodedURL = fullyDecodeString(m_documentURL.string(), decoder);
     225    if (document->decoder())
     226        m_encoding = document->decoder()->encoding();
     227
     228    m_decodedURL = fullyDecodeString(m_documentURL.string(), m_encoding);
    227229    if (m_decodedURL.find(isRequiredForInjection) == notFound)
    228230        m_decodedURL = String();
    … …  
    255257            httpBodyAsString = httpBody->flattenToString();
    256258            if (!httpBodyAsString.isEmpty()) {
    257                 m_decodedHTTPBody = fullyDecodeString(httpBodyAsString, decoder);
     259                m_decodedHTTPBody = fullyDecodeString(httpBodyAsString, m_encoding);
    258260                if (m_decodedHTTPBody.find(isRequiredForInjection) == notFound)
    259261                    m_decodedHTTPBody = String();
    … …  
    271273    if (!m_reportURL.isEmpty()) {
    272274        // May need these for reporting later on.
    273         m_originalURL = m_documentURL;
     275        m_originalURL = m_documentURL.copy();
    274276        m_originalHTTPBody = httpBodyAsString;
    275277    }
    … …  
    508510{
    509511    // Grab a fixed number of characters equal to the length of the token's name plus one (to account for the "<").
    510     return fullyDecodeString(request.sourceTracker.sourceForToken(request.token), request.decoder).substring(0, request.token.name().size() + 1);
     512    return fullyDecodeString(request.sourceTracker.sourceForToken(request.token), m_encoding).substring(0, request.token.name().size() + 1);
    511513}
    512514
    … …  
    519521    int start = attribute.m_nameRange.m_start - request.token.startIndex();
    520522    int end = attribute.m_valueRange.m_end - request.token.startIndex();
    521     String decodedSnippet = fullyDecodeString(request.sourceTracker.sourceForToken(request.token).substring(start, end - start), request.decoder);
     523    String decodedSnippet = fullyDecodeString(request.sourceTracker.sourceForToken(request.token).substring(start, end - start), m_encoding);
    522524    decodedSnippet.truncate(kMaximumFragmentLengthTarget);
    523525    if (treatment == SrcLikeAttribute) {
    … …  
    627629        }
    628630
    629         result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), request.decoder);
     631        result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding);
    630632        startPosition = foundPosition + 1;
    631633    }
    … …  
    665667}
    666668
     669bool XSSAuditor::isSafeToSendToAnotherThread() const
     670{
     671    return m_documentURL.isSafeToSendToAnotherThread()
     672        && m_originalURL.isSafeToSendToAnotherThread()
     673        && m_originalHTTPBody.isSafeToSendToAnotherThread()
     674        && m_decodedURL.isSafeToSendToAnotherThread()
     675        && m_decodedHTTPBody.isSafeToSendToAnotherThread()
     676        && m_cachedDecodedSnippet.isSafeToSendToAnotherThread()
     677        && m_reportURL.isSafeToSendToAnotherThread();
     678}
     679
    667680} // namespace WebCore

  • trunk/Source/WebCore/html/parser/XSSAuditor.h

    r142004 r142099  
    3030#include "HTTPParsers.h"
    3131#include "SuffixTree.h"
     32#include "TextEncoding.h"
    3233#include <wtf/PassOwnPtr.h>
    3334
    … …  
    3738class HTMLDocumentParser;
    3839class HTMLSourceTracker;
    39 class TextResourceDecoder;
    4040class XSSInfo;
    4141
    4242struct FilterTokenRequest {
    43     FilterTokenRequest(HTMLToken& token, HTMLSourceTracker& sourceTracker, const TextResourceDecoder* decoder, bool shouldAllowCDATA)
     43    FilterTokenRequest(HTMLToken& token, HTMLSourceTracker& sourceTracker, bool shouldAllowCDATA)
    4444        : token(token)
    4545        , sourceTracker(sourceTracker)
    46         , decoder(decoder)
    4746        , shouldAllowCDATA(shouldAllowCDATA)
    4847    { }
    … …  
    5049    HTMLToken& token;
    5150    HTMLSourceTracker& sourceTracker;
    52     const TextResourceDecoder* decoder;
    5351    bool shouldAllowCDATA;
    5452};
    … …  
    6159    void init(Document*);
    6260    PassOwnPtr<XSSInfo> filterToken(const FilterTokenRequest&);
     61    bool isSafeToSendToAnotherThread() const;
    6362
    6463private:
    … …  
    114113    unsigned m_scriptTagNestingLevel;
    115114    KURL m_reportURL;
     115    TextEncoding m_encoding;
    116116};
    117117
Note: See TracChangeset for help on using the changeset viewer.