Changeset 76981 in webkit


Ignore:
Timestamp:
Jan 28, 2011 1:31:06 PM (13 years ago)
Author:
abarth@webkit.org
Message:

2011-01-28 Adam Barth <abarth@webkit.org>

Reviewed by Daniel Bates.

Teach XSSFilter how to filter <script> elements
https://bugs.webkit.org/show_bug.cgi?id=53279

This patch adds the ability for the XSSFilter to block injected
<script> elements. Handling script elements is slightly subtle because
these elements act very differently depending on whether they have a
src attribute.


In the "src case", which check whether the src attribute was present in
the request. In the "non-src case", we check whether the start tag and
the body of the script element was included in the request. Checking
for the whole start tag means we miss out on some attribute splitting
attacks inside of script tags, but that doesn't seem like that big a
deal.

This patch also introduces some amount of state into the XSSFilter
because inline script elements span multiple tokens. There's a lot of
tuning and optimization left in these cases, some of which I've noted
with FIXMEs.

To test this patch, I played around with some of the existing
XSSAuditor tests. Hopefully I'll be able to run the test suite more
systematically in the future.

  • html/parser/HTMLToken.h: (WebCore::HTMLToken::eraseCharacters): (WebCore::HTMLToken::eraseValueOfAttribute):
  • html/parser/XSSFilter.cpp: (WebCore::HTMLNames::hasName): (WebCore::HTMLNames::findAttributeWithName): (WebCore::HTMLNames::isNameOfScriptCarryingAttribute): (WebCore::XSSFilter::XSSFilter): (WebCore::XSSFilter::filterToken): (WebCore::XSSFilter::filterTokenAfterScriptStartTag): (WebCore::XSSFilter::filterScriptToken): (WebCore::XSSFilter::snippetForRange): (WebCore::XSSFilter::snippetForAttribute):
  • html/parser/XSSFilter.h:
Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r76980 r76981  
     12011-01-28  Adam Barth  <abarth@webkit.org>
     2
     3        Reviewed by Daniel Bates.
     4
     5        Teach XSSFilter how to filter <script> elements
     6        https://bugs.webkit.org/show_bug.cgi?id=53279
     7
     8        This patch adds the ability for the XSSFilter to block injected
     9        <script> elements.  Handling script elements is slightly subtle because
     10        these elements act very differently depending on whether they have a
     11        src attribute.
     12       
     13        In the "src case", which check whether the src attribute was present in
     14        the request.  In the "non-src case", we check whether the start tag and
     15        the body of the script element was included in the request.  Checking
     16        for the whole start tag means we miss out on some attribute splitting
     17        attacks inside of script tags, but that doesn't seem like that big a
     18        deal.
     19
     20        This patch also introduces some amount of state into the XSSFilter
     21        because inline script elements span multiple tokens.  There's a lot of
     22        tuning and optimization left in these cases, some of which I've noted
     23        with FIXMEs.
     24
     25        To test this patch, I played around with some of the existing
     26        XSSAuditor tests.  Hopefully I'll be able to run the test suite more
     27        systematically in the future.
     28
     29        * html/parser/HTMLToken.h:
     30        (WebCore::HTMLToken::eraseCharacters):
     31        (WebCore::HTMLToken::eraseValueOfAttribute):
     32        * html/parser/XSSFilter.cpp:
     33        (WebCore::HTMLNames::hasName):
     34        (WebCore::HTMLNames::findAttributeWithName):
     35        (WebCore::HTMLNames::isNameOfScriptCarryingAttribute):
     36        (WebCore::XSSFilter::XSSFilter):
     37        (WebCore::XSSFilter::filterToken):
     38        (WebCore::XSSFilter::filterTokenAfterScriptStartTag):
     39        (WebCore::XSSFilter::filterScriptToken):
     40        (WebCore::XSSFilter::snippetForRange):
     41        (WebCore::XSSFilter::snippetForAttribute):
     42        * html/parser/XSSFilter.h:
     43
    1442011-01-28  Adam Barth  <abarth@webkit.org>
    245
  • trunk/Source/WebCore/html/parser/HTMLToken.h

    r76835 r76981  
    245245        ASSERT(m_type == StartTag || m_type == EndTag || m_type == DOCTYPE);
    246246        return m_data;
     247    }
     248
     249    void eraseCharacters()
     250    {
     251        ASSERT(m_type == Character);
     252        m_data.clear();
     253    }
     254
     255    void eraseValueOfAttribute(size_t i)
     256    {
     257        ASSERT(m_type == StartTag || m_type == EndTag);
     258        m_attributes[i].m_value.clear();
    247259    }
    248260
  • trunk/Source/WebCore/html/parser/XSSFilter.cpp

    r76980 r76981  
    2929#include "Document.h"
    3030#include "HTMLDocumentParser.h"
     31#include "HTMLNames.h"
    3132#include "TextEncoding.h"
    3233#include "TextResourceDecoder.h"
     
    3839namespace WebCore {
    3940
     41using namespace HTMLNames;
     42
    4043namespace {
     44
     45bool hasName(const HTMLToken& token, const QualifiedName& name)
     46{
     47    return equalIgnoringNullity(token.name(), static_cast<const String&>(name.localName()));
     48}
     49
     50bool findAttributeWithName(const HTMLToken& token, const QualifiedName& name, size_t& indexOfMatchingAttribute)
     51{
     52    for (size_t i = 0; i < token.attributes().size(); ++i) {
     53        if (equalIgnoringNullity(token.attributes().at(i).m_name, name.localName())) {
     54            indexOfMatchingAttribute = i;
     55            return true;
     56        }
     57    }
     58    return false;
     59}
    4160
    4261bool isNameOfScriptCarryingAttribute(const Vector<UChar, 32>& name)
     
    4564    if (name.size() < lengthOfShortestScriptCarryingAttribute)
    4665        return false;
    47     if (name[0] != 'o' && name[0] != 'O')
    48         return false;
    49     if (name[1] != 'n' && name[0] != 'N')
    50         return false;
    51     return true;
     66    return name[0] == 'o' && name[1] == 'n';
    5267}
    5368
     
    6984XSSFilter::XSSFilter(HTMLDocumentParser* parser)
    7085    : m_parser(parser)
     86    , m_state(Initial)
    7187{
    7288    ASSERT(m_parser);
     
    7995    return;
    8096#else
     97    switch (m_state) {
     98    case Initial:
     99        break;
     100    case AfterScriptStartTag:
     101        filterTokenAfterScriptStartTag(token);
     102        ASSERT(m_state == Initial);
     103        m_cachedSnippet = String();
     104        return;
     105    }
     106
    81107    if (token.type() != HTMLToken::StartTag)
    82108        return;
    83109
    84     HTMLToken::AttributeList::const_iterator iter = token.attributes().begin();
    85     for (; iter != token.attributes().end(); ++iter) {
    86         if (!isNameOfScriptCarryingAttribute(iter->m_name))
     110    if (hasName(token, scriptTag)) {
     111        filterScriptToken(token);
     112        return;
     113    }
     114
     115    for (size_t i = 0; i < token.attributes().size(); ++i) {
     116        const HTMLToken::Attribute& attribute = token.attributes().at(i);
     117        if (!isNameOfScriptCarryingAttribute(attribute.m_name))
    87118            continue;
    88         if (!isContainedInRequest(snippetForAttribute(token, *iter)))
     119        if (!isContainedInRequest(snippetForAttribute(token, attribute)))
    89120            continue;
    90         iter->m_value.clear();
     121        token.eraseValueOfAttribute(i);
    91122    }
    92123#endif
     124}
     125
     126void XSSFilter::filterTokenAfterScriptStartTag(HTMLToken& token)
     127{
     128    ASSERT(m_state == AfterScriptStartTag);
     129    m_state = Initial;
     130
     131    if (token.type() != HTMLToken::Character) {
     132        ASSERT(token.type() == HTMLToken::EndTag || token.type() == HTMLToken::EndOfFile);
     133        return;
     134    }
     135
     136    int start = 0;
     137    // FIXME: We probably want to grab only the first few characters of the
     138    //        contents of the script element.
     139    int end = token.endIndex() - token.startIndex();
     140    if (isContainedInRequest(m_cachedSnippet + snippetForRange(token, start, end))) {
     141        token.eraseCharacters();
     142        token.appendToCharacter(' '); // Technically, character tokens can't be empty.
     143    }
     144}
     145
     146void XSSFilter::filterScriptToken(HTMLToken& token)
     147{
     148    ASSERT(m_state == Initial);
     149    ASSERT(token.type() == HTMLToken::StartTag);
     150    ASSERT(hasName(token, scriptTag));
     151
     152    size_t indexOfFirstSrcAttribute;
     153    if (findAttributeWithName(token, srcAttr, indexOfFirstSrcAttribute)) {
     154        const HTMLToken::Attribute& srcAttribute = token.attributes().at(indexOfFirstSrcAttribute);
     155        if (isContainedInRequest(snippetForAttribute(token, srcAttribute)))
     156            token.eraseValueOfAttribute(indexOfFirstSrcAttribute);
     157        return;
     158    }
     159
     160    m_state = AfterScriptStartTag;
     161    m_cachedSnippet = m_parser->sourceForToken(token);
     162}
     163
     164String XSSFilter::snippetForRange(const HTMLToken& token, int start, int end)
     165{
     166    // FIXME: There's an extra allocation here that we could save by
     167    //        passing the range to the parser.
     168    return m_parser->sourceForToken(token).substring(start, end - start);
    93169}
    94170
     
    99175    // FIXME: We probably want to grab only the first few characters of the attribute value.
    100176    int end = attribute.m_valueRange.m_end - token.startIndex();
    101 
    102     // FIXME: There's an extra allocation here that we could save by
    103     //        passing the range to the parser.
    104     return m_parser->sourceForToken(token).substring(start, end - start);
     177    return snippetForRange(token, start, end);
    105178}
    106179
  • trunk/Source/WebCore/html/parser/XSSFilter.h

    r76980 r76981  
    4141
    4242private:
     43    enum State {
     44        Initial,
     45        AfterScriptStartTag,
     46    };
     47
     48    void filterTokenAfterScriptStartTag(HTMLToken&);
     49    void filterScriptToken(HTMLToken&);
     50
     51    String snippetForRange(const HTMLToken&, int start, int end);
    4352    String snippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
     53
    4454    bool isContainedInRequest(const String&);
    4555
    4656    HTMLDocumentParser* m_parser;
     57    State m_state;
     58    String m_cachedSnippet;
    4759};
    4860
Note: See TracChangeset for help on using the changeset viewer.