Changeset 95366 in webkit


Ignore:
Timestamp:
Sep 16, 2011 10:16:07 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

Make XSSAuditor truncate inline snippets at a reasonable length before comparison
respecting boundaries of multiply urlencoded sequences.
https://bugs.webkit.org/show_bug.cgi?id=68092

Patch by Tom Sepez <tsepez@chromium.org> on 2011-09-16
Reviewed by Adam Barth.

Source/WebCore:

Test: http/tests/security/xssAuditor/property-escape-long.html

  • html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::filterTokenAfterScriptStartTag):
(WebCore::XSSAuditor::eraseDangerousAttributesIfInjected):
(WebCore::XSSAuditor::eraseAttributeIfInjected):
(WebCore::XSSAuditor::decodedSnippetForAttribute):
(WebCore::XSSAuditor::isContainedInRequest):

  • html/parser/XSSAuditor.h:

LayoutTests:

  • http/tests/security/xssAuditor/property-escape-long-expected.txt: Added.
  • http/tests/security/xssAuditor/property-escape-long.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r95362 r95366  
     12011-09-16  Tom Sepez  <tsepez@chromium.org>
     2
     3        Make XSSAuditor truncate inline snippets at a reasonable length before comparison
     4        respecting boundaries of multiply urlencoded sequences.
     5        https://bugs.webkit.org/show_bug.cgi?id=68092
     6       
     7        Reviewed by Adam Barth.
     8
     9        * http/tests/security/xssAuditor/property-escape-long-expected.txt: Added.
     10        * http/tests/security/xssAuditor/property-escape-long.html: Added.
     11
    1122011-09-16  Dmitry Lomov  <dslomov@google.com>
    213
  • trunk/Source/WebCore/ChangeLog

    r95365 r95366  
     12011-09-16  Tom Sepez  <tsepez@chromium.org>
     2
     3        Make XSSAuditor truncate inline snippets at a reasonable length before comparison
     4        respecting boundaries of multiply urlencoded sequences.
     5        https://bugs.webkit.org/show_bug.cgi?id=68092
     6       
     7        Reviewed by Adam Barth.
     8
     9        Test: http/tests/security/xssAuditor/property-escape-long.html
     10
     11        * html/parser/XSSAuditor.cpp:
     12        (WebCore::XSSAuditor::filterTokenAfterScriptStartTag):
     13        (WebCore::XSSAuditor::eraseDangerousAttributesIfInjected):
     14        (WebCore::XSSAuditor::eraseAttributeIfInjected):
     15        (WebCore::XSSAuditor::decodedSnippetForAttribute):
     16        (WebCore::XSSAuditor::isContainedInRequest):
     17        * html/parser/XSSAuditor.h:
     18
    1192011-09-16  Shawn Singh  <shawnsingh@chromium.org>
    220
  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r95065 r95366  
    310310    //        contents of the script element.
    311311    int end = token.endIndex() - token.startIndex();
    312     if (isContainedInRequest(m_cachedSnippet + snippetForRange(token, start, end))) {
     312    String snippet = m_cachedSnippet + snippetForRange(token, start, end);
     313    if (isContainedInRequest(fullyDecodeString(snippet, m_parser->document()->decoder()))) {
    313314        token.eraseCharacters();
    314315        token.appendToCharacter(' '); // Technically, character tokens can't be empty.
     
    441442        if (!isInlineEventHandler && !valueContainsJavaScriptURL)
    442443            continue;
    443         if (!isContainedInRequest(snippetForAttribute(token, attribute)))
     444        // Beware of trailing characters which came from the page itself, not the
     445        // injected vector. Excluding the terminating character covers common cases
     446        // where the page immediately ends the attribute, but doesn't cover more
     447        // complex cases where there is other page data following the injection.
     448        // Generally, these won't parse as javascript, so the injected vector
     449        // typically excludes them from consideration via a single-line comment or
     450        // by enclosing them in a string literal terminated later by the page's own
     451        // closing punctuation. Since the snippet has not been parsed, the vector
     452        // may also try to introduce these via entities. As a result, we'd like to
     453        // stop before the first "//", the first entity, or the first quote not
     454        // immediately following the first equals sign (taking whitespace into
     455        // consideration). To keep things simpler, we don't try to distinguish
     456        // between entity-introducing amperands vs. other uses, nor do we bother to
     457        // check for a second slash for a comment, stoping instead on any ampersand
     458        // or slash.
     459        String decodedSnippet = decodedSnippetForAttribute(token, attribute);
     460        size_t position;
     461        if ((position = decodedSnippet.find("=")) != notFound
     462            && (position = decodedSnippet.find(isNotHTMLSpace, position + 1)) != notFound
     463            && (position = decodedSnippet.find(isTerminatingCharacter, isHTMLQuote(decodedSnippet[position]) ? position + 1 : position)) != notFound) {
     464            decodedSnippet.truncate(position);
     465        }
     466        if (!isContainedInRequest(decodedSnippet))
    444467            continue;
    445468        token.eraseValueOfAttribute(i);
     
    456479    if (findAttributeWithName(token, attributeName, indexOfAttribute)) {
    457480        const HTMLToken::Attribute& attribute = token.attributes().at(indexOfAttribute);
    458         if (isContainedInRequest(snippetForAttribute(token, attribute))) {
     481        if (isContainedInRequest(decodedSnippetForAttribute(token, attribute))) {
    459482            if (attributeName == srcAttr && isSameOriginResource(String(attribute.m_value.data(), attribute.m_value.size())))
    460483                return false;
     
    477500}
    478501
    479 String XSSAuditor::snippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
    480 {
     502String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
     503{
     504    const size_t kMaximumSnippetLength = 100;
     505
    481506    // The range doesn't inlcude the character which terminates the value. So,
    482507    // for an input of |name="value"|, the snippet is |name="value|. For an
     
    485510    int start = attribute.m_nameRange.m_start - token.startIndex();
    486511    int end = attribute.m_valueRange.m_end - token.startIndex();
    487     String snippet = snippetForRange(token, start, end);
    488 
    489     // Beware of trailing characters which came from the page itself, not the
    490     // injected vector. Excluding the terminating character covers common cases
    491     // where the page immediately ends the attribute, but doesn't cover more
    492     // complex cases where there is other page data following the injection.
    493     // Generally, these won't parse as javascript, so the injected vector
    494     // typically excludes them from consideration via a single-line comment or
    495     // by enclosing them in a string literal terminated later by the page's own
    496     // closing punctuation. Since the snippet has not been parsed, the vector
    497     // may also try to introduce these via entities. As a result, we'd like to
    498     // stop before the first "//", the first entity, or the first quote not
    499     // immediately following the first equals sign (taking whitespace into
    500     // consideration). To keep things simpler, we don't try to distinguish
    501     // between entity-introducing amperands vs. other uses, nor do we bother to
    502     // check for a second slash for a comment, stoping instead on any ampersand
    503     // or slash.
    504     size_t position;
    505     if ((position = snippet.find("=")) != notFound
    506         && (position = snippet.find(isNotHTMLSpace, position + 1)) != notFound
    507         && (position = snippet.find(isTerminatingCharacter, isHTMLQuote(snippet[position]) ? position + 1 : position)) != notFound) {
    508         snippet.truncate(position);
    509     }
    510 
    511     return snippet;
    512 }
    513 
    514 bool XSSAuditor::isContainedInRequest(const String& snippet)
    515 {
    516     ASSERT(!snippet.isEmpty());
    517     TextResourceDecoder* decoder = m_parser->document()->decoder();
    518     String decodedSnippet = fullyDecodeString(snippet, decoder);
     512    String decodedSnippet = fullyDecodeString(snippetForRange(token, start, end), m_parser->document()->decoder());
     513    decodedSnippet.truncate(kMaximumSnippetLength);
     514    return decodedSnippet;
     515}
     516
     517bool XSSAuditor::isContainedInRequest(const String& decodedSnippet)
     518{
    519519    if (decodedSnippet.isEmpty())
    520520        return false;
  • trunk/Source/WebCore/html/parser/XSSAuditor.h

    r87708 r95366  
    6868
    6969    String snippetForRange(const HTMLToken&, int start, int end);
    70     String snippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
     70    String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
    7171
    7272    bool isContainedInRequest(const String&);
Note: See TracChangeset for help on using the changeset viewer.