Changeset 95774 in webkit


Ignore:
Timestamp:
Sep 22, 2011 7:28:31 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

Make XSSAuditor extract meaningful snippet from script blocks for comparison
against the URL when checking for reflection. Avoids getting caugh up in
trailing comments.
https://bugs.webkit.org/show_bug.cgi?id=68094

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

Source/WebCore:

Tests: http/tests/security/xssAuditor/script-tag-with-trailing-comment.html

http/tests/security/xssAuditor/script-tag-with-trailing-comment2.html
http/tests/security/xssAuditor/script-tag-with-trailing-comment3.html

  • html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::filterTokenAfterScriptStartTag):
(WebCore::XSSAuditor::extractCodeFragment):

  • html/parser/XSSAuditor.h:

LayoutTests:

  • http/tests/security/xssAuditor/resources/echo-intertag.pl:
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment.html: Added.
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment2-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment2.html: Added.
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment3-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-with-trailing-comment3.html: Added.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r95771 r95774  
     12011-09-22  Tom Sepez  <tsepez@chromium.org>
     2
     3        Make XSSAuditor extract meaningful snippet from script blocks for comparison
     4        against the URL when checking for reflection.  Avoids getting caugh up in
     5        trailing comments.
     6        https://bugs.webkit.org/show_bug.cgi?id=68094
     7
     8        Reviewed by Adam Barth.
     9
     10        * http/tests/security/xssAuditor/resources/echo-intertag.pl:
     11        * http/tests/security/xssAuditor/script-tag-with-trailing-comment-expected.txt: Added.
     12        * http/tests/security/xssAuditor/script-tag-with-trailing-comment.html: Added.
     13        * http/tests/security/xssAuditor/script-tag-with-trailing-comment2-expected.txt: Added.
     14        * http/tests/security/xssAuditor/script-tag-with-trailing-comment2.html: Added.
     15        * http/tests/security/xssAuditor/script-tag-with-trailing-comment3-expected.txt: Added.
     16        * http/tests/security/xssAuditor/script-tag-with-trailing-comment3.html: Added.
     17
    1182011-09-22  Ben Wells  <benwells@chromium.org>
    219
  • trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl

    r95451 r95774  
    3636    print $cgi->param('clutter');
    3737}
     38if ($cgi->param('q2')) {
     39    print $cgi->param('q2');
     40}
    3841if ($cgi->param('notifyDone')) {
    3942    print "<script>\n";
  • trunk/Source/WebCore/ChangeLog

    r95768 r95774  
     12011-09-22  Tom Sepez  <tsepez@chromium.org>
     2
     3        Make XSSAuditor extract meaningful snippet from script blocks for comparison
     4        against the URL when checking for reflection.  Avoids getting caugh up in
     5        trailing comments.
     6        https://bugs.webkit.org/show_bug.cgi?id=68094
     7
     8        Reviewed by Adam Barth.
     9
     10        Tests: http/tests/security/xssAuditor/script-tag-with-trailing-comment.html
     11               http/tests/security/xssAuditor/script-tag-with-trailing-comment2.html
     12               http/tests/security/xssAuditor/script-tag-with-trailing-comment3.html
     13
     14        * html/parser/XSSAuditor.cpp:
     15        (WebCore::XSSAuditor::filterTokenAfterScriptStartTag):
     16        (WebCore::XSSAuditor::extractCodeFragment):
     17        * html/parser/XSSAuditor.h:
     18
    1192011-09-22  Nate Chapin  <japhet@chromium.org>
    220
  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r95366 r95774  
    8181}
    8282
     83static bool isHTMLNewline(UChar c)
     84{
     85    return (c == '\n' || c == '\r');
     86}
     87
     88static bool startsHTMLCommentAt(const String& string, size_t start)
     89{
     90    return (start + 3 < string.length() && string[start] == '<' && string[start+1] == '!' && string[start+2] == '-' && string[start+3] == '-');
     91}   
     92
     93static bool startsSingleLineCommentAt(const String& string, size_t start)
     94{
     95    return (start + 1 < string.length() && string[start] == '/' && string[start+1] == '/');
     96}   
     97
     98static bool startsMultiLineCommentAt(const String& string, size_t start)
     99{
     100    return (start + 1 < string.length() && string[start] == '/' && string[start+1] == '*');
     101}
     102
    83103static bool hasName(const HTMLToken& token, const QualifiedName& name)
    84104{
     
    306326    }
    307327
    308     int start = 0;
    309     // FIXME: We probably want to grab only the first few characters of the
    310     //        contents of the script element.
    311     int end = token.endIndex() - token.startIndex();
    312     String snippet = m_cachedSnippet + snippetForRange(token, start, end);
    313     if (isContainedInRequest(fullyDecodeString(snippet, m_parser->document()->decoder()))) {
    314         token.eraseCharacters();
    315         token.appendToCharacter(' '); // Technically, character tokens can't be empty.
    316         return true;
     328    TextResourceDecoder* decoder = m_parser->document()->decoder();
     329    if (isContainedInRequest(fullyDecodeString(m_cachedSnippet, decoder))) {
     330        int start = 0;
     331        int end = token.endIndex() - token.startIndex();
     332        String snippet = snippetForJavaScript(snippetForRange(token, start, end));
     333        if (isContainedInRequest(fullyDecodeString(snippet, decoder))) {
     334            token.eraseCharacters();
     335            token.appendToCharacter(' '); // Technically, character tokens can't be empty.
     336            return true;
     337        }
    317338    }
    318339    return false;
     
    538559}
    539560
    540 }
     561
     562String XSSAuditor::snippetForJavaScript(const String& string)
     563{
     564    const size_t kMaximumFragmentLengthTarget = 100;
     565
     566    size_t startPosition = 0;
     567    size_t endPosition = string.length();
     568    size_t foundPosition = notFound;
     569
     570    // Skip over initial comments to find start of code.
     571    while (startPosition < endPosition) {
     572        while (startPosition < endPosition && isHTMLSpace(string[startPosition]))
     573            startPosition++;
     574        if (startsHTMLCommentAt(string, startPosition) || startsSingleLineCommentAt(string, startPosition)) {
     575            while (startPosition < endPosition && !isHTMLNewline(string[startPosition]))
     576                startPosition++;
     577        } else if (startsMultiLineCommentAt(string, startPosition)) {
     578            if ((foundPosition = string.find("*/", startPosition)) != notFound)
     579                startPosition = foundPosition + 2;
     580            else
     581                startPosition = endPosition;
     582        } else
     583            break;
     584    }
     585
     586    // Stop at next comment or when we exceed the maximum length target. After hitting the
     587    // length target, we can only stop at a point where we know we are not in the middle of
     588    // a %-escape sequence. A simple way to do this is to break on whitespace only.               
     589    for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
     590        if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
     591            endPosition = foundPosition + 2;
     592            break;
     593        }
     594        if (startsHTMLCommentAt(string, foundPosition)) {
     595            endPosition = foundPosition + 4;
     596            break;
     597        }
     598        if (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition])) {
     599            endPosition = foundPosition;
     600            break;
     601        }
     602    }
     603   
     604    return string.substring(startPosition, endPosition - startPosition);
     605}
     606
     607} // namespace WebCore
  • trunk/Source/WebCore/html/parser/XSSAuditor.h

    r95366 r95774  
    6868
    6969    String snippetForRange(const HTMLToken&, int start, int end);
     70    String snippetForJavaScript(const String&);
    7071    String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
    7172
Note: See TracChangeset for help on using the changeset viewer.