Changeset 95065 in webkit


Ignore:
Timestamp:
Sep 13, 2011 6:19:04 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

Fix XSS auditor bypass when inline handlers contain comments.
​https://bugs.webkit.org/show_bug.cgi?id=27895

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

Source/WebCore:

Tests: http/tests/security/xssAuditor/property-escape-comment.html

http/tests/security/xssAuditor/property-escape-entity.html
http/tests/security/xssAuditor/property-escape-quote.html

  • html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::snippetForAttribute):

LayoutTests:

  • http/tests/security/xssAuditor/malformed-HTML-expected.txt:
  • http/tests/security/xssAuditor/open-attribute-body-expected.txt:
  • http/tests/security/xssAuditor/open-event-handler-iframe-expected.txt:
  • http/tests/security/xssAuditor/property-escape-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/property-escape-comment.html: Added.
  • http/tests/security/xssAuditor/property-escape-entity-expected.txt: Added.
  • http/tests/security/xssAuditor/property-escape-entity.html: Added.
  • http/tests/security/xssAuditor/property-escape-quote-expected.txt: Added.
  • http/tests/security/xssAuditor/property-escape-quote.html: Added.
  • http/tests/security/xssAuditor/resources/echo-property.pl:
Location:
trunk
Files:
6 added
7 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r95064 r95065  
     12011-09-13  Tom Sepez  <tsepez@chromium.org>
     2
     3        Fix XSS auditor bypass when inline handlers contain comments.
     4        https://bugs.webkit.org/show_bug.cgi?id=27895
     5
     6        Reviewed by Adam Barth.
     7
     8        * http/tests/security/xssAuditor/malformed-HTML-expected.txt:
     9        * http/tests/security/xssAuditor/open-attribute-body-expected.txt:
     10        * http/tests/security/xssAuditor/open-event-handler-iframe-expected.txt:
     11        * http/tests/security/xssAuditor/property-escape-comment-expected.txt: Added.
     12        * http/tests/security/xssAuditor/property-escape-comment.html: Added.
     13        * http/tests/security/xssAuditor/property-escape-entity-expected.txt: Added.
     14        * http/tests/security/xssAuditor/property-escape-entity.html: Added.
     15        * http/tests/security/xssAuditor/property-escape-quote-expected.txt: Added.
     16        * http/tests/security/xssAuditor/property-escape-quote.html: Added.
     17        * http/tests/security/xssAuditor/resources/echo-property.pl:
     18
    1192011-09-13  Fumitoshi Ukai  <ukai@chromium.org>
    220

  • trunk/LayoutTests/http/tests/security/xssAuditor/malformed-HTML-expected.txt

    r61234 r95065  
     1CONSOLE MESSAGE: line 1: Refused to execute a JavaScript script. Source code of script found within request.
     2
    13
    24

  • trunk/LayoutTests/http/tests/security/xssAuditor/open-attribute-body-expected.txt

    r78776 r95065  
    1 ALERT: /XSS/
     1CONSOLE MESSAGE: line 1: Refused to execute a JavaScript script. Source code of script found within request.
    22
     3

  • trunk/LayoutTests/http/tests/security/xssAuditor/open-event-handler-iframe-expected.txt

    r78776 r95065  
    1 ALERT: /XSS/
     1CONSOLE MESSAGE: line 1: Refused to execute a JavaScript script. Source code of script found within request.
    22
     3

  • trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-property.pl

    r44985 r95065  
    1111print "<body foo=\"";
    1212print $cgi->param('q');
     13if ($cgi->param('clutter')) {
     14    print $cgi->param('clutter');
     15}
    1316print "\">\n";
    1417print "</body>\n";

  • trunk/Source/WebCore/ChangeLog

    r95063 r95065  
     12011-09-13  Tom Sepez  <tsepez@chromium.org>
     2
     3        Fix XSS auditor bypass when inline handlers contain comments.
     4        https://bugs.webkit.org/show_bug.cgi?id=27895
     5
     6        Reviewed by Adam Barth.
     7
     8        Tests: http/tests/security/xssAuditor/property-escape-comment.html
     9               http/tests/security/xssAuditor/property-escape-entity.html
     10               http/tests/security/xssAuditor/property-escape-quote.html
     11
     12        * html/parser/XSSAuditor.cpp:
     13        (WebCore::XSSAuditor::snippetForAttribute):
     14
    1152011-09-13  Kentaro Hara  <haraken@google.com>
    216

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

    r94828 r95065  
    7171}
    7272
     73static bool isTerminatingCharacter(UChar c)
     74{
     75    return (c == '&' || c == '/' || c == '"' || c == '\'');
     76}
     77
     78static bool isHTMLQuote(UChar c)
     79{
     80    return (c == '"' || c == '\'');
     81}
     82
    7383static bool hasName(const HTMLToken& token, const QualifiedName& name)
    7484{
    … …  
    469479String XSSAuditor::snippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
    470480{
     481    // The range doesn't inlcude the character which terminates the value. So,
     482    // for an input of |name="value"|, the snippet is |name="value|. For an
     483    // unquoted input of |name=value |, the snippet is |name=value|.
    471484    // FIXME: We should grab one character before the name also.
    472485    int start = attribute.m_nameRange.m_start - token.startIndex();
    473     // FIXME: We probably want to grab only the first few characters of the attribute value.
    474486    int end = attribute.m_valueRange.m_end - token.startIndex();
    475     return snippetForRange(token, start, end);
     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;
    476512}
    477513
Note: See TracChangeset for help on using the changeset viewer.