Changeset 49644 in webkit


Ignore:
Timestamp:
Oct 15, 2009 12:04:57 PM (15 years ago)
Author:
dbates@webkit.org
Message:

2009-10-15 Daniel Bates <​dbates@webkit.org>

Reviewed by Adam Barth.

​https://bugs.webkit.org/show_bug.cgi?id=27895


Fixes an issue in which injecting an inline event handler whose value ends in a single-line
JavaScript comment can bypass the XSSAuditor. Similarly fixes this issue with respect to
the HTML Base element, HTML Object element, inline and external script tags, and
JavaScript multi-line variants of all of these attacks.

Tests: http/tests/security/xssAuditor/base-href-comment.html

http/tests/security/xssAuditor/iframe-javascript-url-comment.html
http/tests/security/xssAuditor/img-onerror-HTML-comment.html
http/tests/security/xssAuditor/img-onerror-comment.html
http/tests/security/xssAuditor/object-tag-comment.html
http/tests/security/xssAuditor/script-tag-comment-HTML-entity.html
http/tests/security/xssAuditor/script-tag-comment.html
http/tests/security/xssAuditor/script-tag-with-source-comment.html

  • page/XSSAuditor.cpp: Added constant minAttackLength. (WebCore::XSSAuditor::canEvaluate): (WebCore::XSSAuditor::canEvaluateJavaScriptURL): (WebCore::XSSAuditor::canCreateInlineEventListener): (WebCore::XSSAuditor::canLoadExternalScriptFromSrc): (WebCore::XSSAuditor::canLoadObject): (WebCore::XSSAuditor::canSetBaseElementURL): (WebCore::XSSAuditor::findInRequest): Added parameter context. Only looks at up to minAttackLength of script code plus context (if any).
  • page/XSSAuditor.h:

2009-10-15 Daniel Bates <​dbates@webkit.org>

Reviewed by Adam Barth.

​https://bugs.webkit.org/show_bug.cgi?id=27895


Tests that inline event handlers whose value ends in a single-line JavaScript
comment cannot bypass the XSSAuditor. Also tests that the XSSAuditor prevents
similar attack vectors with respect to the HTML Base element, HTML Object
element, and external JavaScripts.

  • http/tests/security/xssAuditor/base-href-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/base-href-comment.html: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-comment.html: Added.
  • http/tests/security/xssAuditor/img-onerror-HTML-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/img-onerror-HTML-comment.html: Added.
  • http/tests/security/xssAuditor/img-onerror-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/img-onerror-comment.html: Added.
  • http/tests/security/xssAuditor/object-tag-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/object-tag-comment.html: Added.
  • http/tests/security/xssAuditor/resources/echo-before-image.pl: Added.
  • http/tests/security/xssAuditor/resources/echo-head-base-href-comment.pl: Added.
  • http/tests/security/xssAuditor/script-tag-comment-HTML-entity-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-comment-HTML-entity.html: Added.
  • http/tests/security/xssAuditor/script-tag-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-comment.html: Added.
  • http/tests/security/xssAuditor/script-tag-with-source-comment-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-with-source-comment.html: Added.
Location:
trunk
Files:
18 added
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r49640 r49644  
     12009-10-15  Daniel Bates  <dbates@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=27895
     6       
     7        Tests that inline event handlers whose value ends in a single-line JavaScript
     8        comment cannot bypass the XSSAuditor. Also tests that the XSSAuditor prevents
     9        similar attack vectors with respect to the HTML Base element, HTML Object
     10        element, and external JavaScripts.
     11
     12        * http/tests/security/xssAuditor/base-href-comment-expected.txt: Added.
     13        * http/tests/security/xssAuditor/base-href-comment.html: Added.
     14        * http/tests/security/xssAuditor/iframe-javascript-url-comment-expected.txt: Added.
     15        * http/tests/security/xssAuditor/iframe-javascript-url-comment.html: Added.
     16        * http/tests/security/xssAuditor/img-onerror-HTML-comment-expected.txt: Added.
     17        * http/tests/security/xssAuditor/img-onerror-HTML-comment.html: Added.
     18        * http/tests/security/xssAuditor/img-onerror-comment-expected.txt: Added.
     19        * http/tests/security/xssAuditor/img-onerror-comment.html: Added.
     20        * http/tests/security/xssAuditor/object-tag-comment-expected.txt: Added.
     21        * http/tests/security/xssAuditor/object-tag-comment.html: Added.
     22        * http/tests/security/xssAuditor/resources/echo-before-image.pl: Added.
     23        * http/tests/security/xssAuditor/resources/echo-head-base-href-comment.pl: Added.
     24        * http/tests/security/xssAuditor/script-tag-comment-HTML-entity-expected.txt: Added.
     25        * http/tests/security/xssAuditor/script-tag-comment-HTML-entity.html: Added.
     26        * http/tests/security/xssAuditor/script-tag-comment-expected.txt: Added.
     27        * http/tests/security/xssAuditor/script-tag-comment.html: Added.
     28        * http/tests/security/xssAuditor/script-tag-with-source-comment-expected.txt: Added.
     29        * http/tests/security/xssAuditor/script-tag-with-source-comment.html: Added.
     30
    1312009-10-15  Brian Weinstein  <bweinstein@apple.com>
    232

  • trunk/WebCore/ChangeLog

    r49641 r49644  
     12009-10-15  Daniel Bates  <dbates@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=27895
     6       
     7        Fixes an issue in which injecting an inline event handler whose value ends in a single-line
     8        JavaScript comment can bypass the XSSAuditor. Similarly fixes this issue with respect to
     9        the HTML Base element, HTML Object element, inline and external script tags, and
     10        JavaScript multi-line variants of all of these attacks.
     11
     12        Tests: http/tests/security/xssAuditor/base-href-comment.html
     13               http/tests/security/xssAuditor/iframe-javascript-url-comment.html
     14               http/tests/security/xssAuditor/img-onerror-HTML-comment.html
     15               http/tests/security/xssAuditor/img-onerror-comment.html
     16               http/tests/security/xssAuditor/object-tag-comment.html
     17               http/tests/security/xssAuditor/script-tag-comment-HTML-entity.html
     18               http/tests/security/xssAuditor/script-tag-comment.html
     19               http/tests/security/xssAuditor/script-tag-with-source-comment.html
     20
     21        * page/XSSAuditor.cpp: Added constant minAttackLength.
     22        (WebCore::XSSAuditor::canEvaluate):
     23        (WebCore::XSSAuditor::canEvaluateJavaScriptURL):
     24        (WebCore::XSSAuditor::canCreateInlineEventListener):
     25        (WebCore::XSSAuditor::canLoadExternalScriptFromSrc):
     26        (WebCore::XSSAuditor::canLoadObject):
     27        (WebCore::XSSAuditor::canSetBaseElementURL):
     28        (WebCore::XSSAuditor::findInRequest): Added parameter context. Only looks at up
     29        to minAttackLength of script code plus context (if any).
     30        * page/XSSAuditor.h:
     31
    1322009-10-08  Adam Langley  <agl@google.com>
    233

  • trunk/WebCore/page/XSSAuditor.cpp

    r49605 r49644  
    4747namespace WebCore {
    4848
     49// Note, we believe it is sufficient to only look at a substring of 7
     50// characters (or less) of code.  Observe that "alert()" is seven characters
     51// in length.
     52static const unsigned minAttackLength = 7;
     53
    4954static bool isNonCanonicalCharacter(UChar c)
    5055{
    … …  
    106111        return true;
    107112
    108     if (findInRequest(code, false, true)) {
     113    if (findInRequest(String(), code, false, true)) {
    109114        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
    110115        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    119124        return true;
    120125
    121     if (findInRequest(code, true, false, true)) {
     126    if (findInRequest(String(), code, true, false, true)) {
    122127        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
    123128        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    132137        return true;
    133138
    134     if (findInRequest(code, true, true)) {
     139    if (findInRequest(String(), code, true, true)) {
    135140        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
    136141        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    155160        return true;
    156161
    157     if (findInRequest(context + url)) {
     162    if (findInRequest(context, url)) {
    158163        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
    159164        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    168173        return true;
    169174
    170     if (findInRequest(url)) {
     175    if (findInRequest(String(), url)) {
    171176        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request"));
    172177        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    182187   
    183188    KURL baseElementURL(m_frame->document()->url(), url);
    184     if (m_frame->document()->url().host() != baseElementURL.host() && findInRequest(url)) {
     189    if (m_frame->document()->url().host() != baseElementURL.host() && findInRequest(String(), url)) {
    185190        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request"));
    186191        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    256261}
    257262
    258 bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
     263bool XSSAuditor::findInRequest(const String& context, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
    259264                               bool decodeURLEscapeSequencesTwice) const
    260265{
    … …  
    262267    Frame* parentFrame = m_frame->tree()->parent();
    263268    if (parentFrame && m_frame->document()->url() == blankURL())
    264         result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
     269        result = findInRequest(parentFrame, context, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
    265270    if (!result)
    266         result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
     271        result = findInRequest(m_frame, context, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
    267272    return result;
    268273}
    269274
    270 bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
     275bool XSSAuditor::findInRequest(Frame* frame, const String& context, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
    271276                               bool decodeURLEscapeSequencesTwice) const
    272277{
    … …  
    282287
    283288    FormData* formDataObj = frame->loader()->documentLoader()->originalRequest().httpBody();
     289    const bool hasFormData = formDataObj && !formDataObj->isEmpty();
    284290    String pageURL = frame->document()->url().string();
    285 
    286     if (!formDataObj && string.length() >= 2 * pageURL.length()) {
     291   
     292    String canonicalizedString;
     293    if (!hasFormData && string.length() > 2 * pageURL.length()) {
    287294        // Q: Why do we bother to do this check at all?
    288295        // A: Canonicalizing large inline scripts can be expensive.  We want to
    289         //    bail out before the call to canonicalize below, which could
    290         //    result in an unneeded allocation and memcpy.
     296        //    reduce the size of the string before we call canonicalize below,
     297        //    since it could result in an unneeded allocation and memcpy.
    291298        //
    292299        // Q: Why do we multiply by two here?
    … …  
    296303        //    factor of two by sending " characters, which the server
    297304        //    transforms to \".
    298         return false;
    299     }
     305        canonicalizedString = string.substring(0, 2 * pageURL.length());
     306    } else
     307        canonicalizedString = string;
    300308
    301309    if (frame->document()->url().protocolIs("data"))
    302310        return false;
    303311
    304     String canonicalizedString = canonicalize(string);
     312    canonicalizedString = canonicalize(canonicalizedString);
    305313    if (canonicalizedString.isEmpty())
    306314        return false;
    307315
    308     if (string.length() < pageURL.length()) {
    309         // The string can actually fit inside the pageURL.
    310         String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
    311 
    312         if (allowRequestIfNoIllegalURICharacters && (!formDataObj || formDataObj->isEmpty())
    313             && decodedPageURL.find(&isIllegalURICharacter, 0) == -1)
    314             return false; // Injection is impossible because the request does not contain any illegal URI characters.
    315 
    316         if (decodedPageURL.find(canonicalizedString, 0, false) != -1)
    317             return true;  // We've found the smoking gun.
    318     }
    319 
    320     if (formDataObj && !formDataObj->isEmpty()) {
    321         String formData = formDataObj->flattenToString();
    322         if (string.length() < formData.length()) {
    323             // Notice it is sufficient to compare the length of the string to
    324             // the url-encoded POST data because the length of the url-decoded
    325             // code is less than or equal to the length of the url-encoded
    326             // string.
    327             String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
    328             if (decodedFormData.find(canonicalizedString, 0, false) != -1)
    329                 return true;  // We found the string in the POST data.
    330         }
     316    // We only look at the first minAttackLength characters to avoid looking at
     317    // characters the attacker has pulled in from the page using an attack string
     318    // like: <img onerror="alert(/XSS/);//
     319    canonicalizedString = canonicalizedString.substring(0, minAttackLength);
     320
     321    if (!context.isEmpty())
     322        canonicalizedString = context + canonicalizedString;
     323
     324    String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
     325
     326    if (allowRequestIfNoIllegalURICharacters && !hasFormData && decodedPageURL.find(&isIllegalURICharacter, 0) == -1)
     327        return false; // Injection is impossible because the request does not contain any illegal URI characters.
     328
     329    if (decodedPageURL.find(canonicalizedString, 0, false) != -1)
     330        return true;  // We've found the string in the GET data.
     331
     332    if (hasFormData) {
     333        String decodedFormData = m_cache.canonicalizeURL(formDataObj->flattenToString(), frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
     334        if (decodedFormData.find(canonicalizedString, 0, false) != -1)
     335            return true;  // We found the string in the POST data.
    331336    }
    332337

  • trunk/WebCore/page/XSSAuditor.h

    r49434 r49644  
    123123        static String decodeHTMLEntities(const String&, bool leaveUndecodableEntitiesUntouched = true);
    124124
    125         bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
     125        bool findInRequest(const String& context, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
    126126                           bool decodeURLEscapeSequencesTwice = false) const;
    127         bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
     127        bool findInRequest(Frame*, const String& context, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
    128128                           bool decodeURLEscapeSequencesTwice = false) const;
    129129
Note: See TracChangeset for help on using the changeset viewer.