Changeset 97715 in webkit


Ignore:
Timestamp:
Oct 17, 2011 9:37:22 PM (12 years ago)
Author:
commit-queue@webkit.org
Message:

XSSAuditor bypass with remote script ending in ? character
https://bugs.webkit.org/show_bug.cgi?id=70255

Patch by Tom Sepez <tsepez@chromium.org> on 2011-10-17
Reviewed by Daniel Bates.

Fix XSSAuditor bypass where unterminated src="" attribute could pick up
text from page causing failed XSS detection. Constrain match to domain
portions of src attribute only.

Source/WebCore:

Test: http/tests/security/xssAuditor/script-tag-with-source-unterminated.html

  • html/parser/XSSAuditor.cpp:

(WebCore::XSSAuditor::filterScriptToken):
(WebCore::XSSAuditor::filterObjectToken):
(WebCore::XSSAuditor::filterParamToken):
(WebCore::XSSAuditor::filterEmbedToken):
(WebCore::XSSAuditor::filterAppletToken):
(WebCore::XSSAuditor::filterIframeToken):
(WebCore::XSSAuditor::eraseAttributeIfInjected):
(WebCore::XSSAuditor::decodedSnippetForAttribute):

  • html/parser/XSSAuditor.h:

LayoutTests:

  • http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt: Added.
  • http/tests/security/xssAuditor/script-tag-with-source-unterminated.html: Added.
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r97713 r97715  
     12011-10-17  Tom Sepez  <tsepez@chromium.org>
     2
     3        XSSAuditor bypass with remote script ending in ? character
     4        https://bugs.webkit.org/show_bug.cgi?id=70255
     5
     6        Reviewed by Daniel Bates.
     7
     8        Fix XSSAuditor bypass where unterminated src="" attribute could pick up
     9        text from page causing failed XSS detection.  Constrain match to domain
     10        portions of src attribute only.
     11
     12        * http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt: Added.
     13        * http/tests/security/xssAuditor/script-tag-with-source-unterminated.html: Added.
     14
    1152011-10-17  Kent Tamura  <tkent@chromium.org>
    216
  • trunk/Source/WebCore/ChangeLog

    r97714 r97715  
     12011-10-17  Tom Sepez  <tsepez@chromium.org>
     2
     3        XSSAuditor bypass with remote script ending in ? character
     4        https://bugs.webkit.org/show_bug.cgi?id=70255
     5
     6        Reviewed by Daniel Bates.
     7
     8        Fix XSSAuditor bypass where unterminated src="" attribute could pick up
     9        text from page causing failed XSS detection.  Constrain match to domain
     10        portions of src attribute only.
     11
     12        Test: http/tests/security/xssAuditor/script-tag-with-source-unterminated.html
     13
     14        * html/parser/XSSAuditor.cpp:
     15        (WebCore::XSSAuditor::filterScriptToken):
     16        (WebCore::XSSAuditor::filterObjectToken):
     17        (WebCore::XSSAuditor::filterParamToken):
     18        (WebCore::XSSAuditor::filterEmbedToken):
     19        (WebCore::XSSAuditor::filterAppletToken):
     20        (WebCore::XSSAuditor::filterIframeToken):
     21        (WebCore::XSSAuditor::eraseAttributeIfInjected):
     22        (WebCore::XSSAuditor::decodedSnippetForAttribute):
     23        * html/parser/XSSAuditor.h:
     24
    1252011-10-17  Adam Klein  <adamk@chromium.org>
    226
  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r96440 r97715  
    354354    ASSERT(hasName(token, scriptTag));
    355355
    356     if (eraseAttributeIfInjected(token, srcAttr, blankURL().string()))
     356    if (eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute))
    357357        return true;
    358358
     
    370370    bool didBlockScript = false;
    371371
    372     didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string());
     372    didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string(), SrcLikeAttribute);
    373373    didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
    374374    didBlockScript |= eraseAttributeIfInjected(token, classidAttr);
     
    393393        return false;
    394394
    395     return eraseAttributeIfInjected(token, valueAttr, blankURL().string());
     395    return eraseAttributeIfInjected(token, valueAttr, blankURL().string(), SrcLikeAttribute);
    396396}
    397397
     
    404404    bool didBlockScript = false;
    405405
    406     didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string());
     406    didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute);
    407407    didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
    408408
     
    418418    bool didBlockScript = false;
    419419
    420     didBlockScript |= eraseAttributeIfInjected(token, codeAttr);
     420    didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
    421421    didBlockScript |= eraseAttributeIfInjected(token, objectAttr);
    422422
     
    430430    ASSERT(hasName(token, iframeTag));
    431431
    432     return eraseAttributeIfInjected(token, srcAttr);
     432    return eraseAttributeIfInjected(token, srcAttr, String(), SrcLikeAttribute);
    433433}
    434434
     
    503503}
    504504
    505 bool XSSAuditor::eraseAttributeIfInjected(HTMLToken& token, const QualifiedName& attributeName, const String& replacementValue)
     505bool XSSAuditor::eraseAttributeIfInjected(HTMLToken& token, const QualifiedName& attributeName, const String& replacementValue, AttributeKind treatment)
    506506{
    507507    size_t indexOfAttribute;
    508508    if (findAttributeWithName(token, attributeName, indexOfAttribute)) {
    509509        const HTMLToken::Attribute& attribute = token.attributes().at(indexOfAttribute);
    510         if (isContainedInRequest(decodedSnippetForAttribute(token, attribute))) {
     510        if (isContainedInRequest(decodedSnippetForAttribute(token, attribute, treatment))) {
    511511            if (attributeName == srcAttr && isSameOriginResource(String(attribute.m_value.data(), attribute.m_value.size())))
    512512                return false;
     
    529529}
    530530
    531 String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
     531String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute, AttributeKind treatment)
    532532{
    533533    const size_t kMaximumSnippetLength = 100;
     
    541541    String decodedSnippet = fullyDecodeString(snippetForRange(token, start, end), m_parser->document()->decoder());
    542542    decodedSnippet.truncate(kMaximumSnippetLength);
     543    if (treatment == SrcLikeAttribute) {
     544        int slashCount;
     545        size_t currentLength;
     546        // Characters following the first ?, #, or third slash may come from
     547        // the page itself and can be merely ignored by an attacker's server
     548        // when a remote script or script-like resource is requested.
     549        for (slashCount = 0, currentLength = 0; currentLength < decodedSnippet.length(); ++currentLength) {
     550            if (decodedSnippet[currentLength] == '?' || decodedSnippet[currentLength] == '#'
     551                || ((decodedSnippet[currentLength] == '/' || decodedSnippet[currentLength] == '\\') && ++slashCount > 2)) {
     552                decodedSnippet.truncate(currentLength);
     553                break;
     554            }
     555        }
     556    }
    543557    return decodedSnippet;
    544558}
     
    566580    return (m_parser->document()->url().host() == resourceURL.host() && resourceURL.query().isEmpty());
    567581}
    568 
    569582
    570583String XSSAuditor::snippetForJavaScript(const String& string)
  • trunk/Source/WebCore/html/parser/XSSAuditor.h

    r95901 r97715  
    4949    };
    5050
     51    enum AttributeKind {
     52        NormalAttribute,
     53        SrcLikeAttribute
     54    };
     55
    5156    void init();
    5257
     
    6570
    6671    bool eraseDangerousAttributesIfInjected(HTMLToken&);
    67     bool eraseAttributeIfInjected(HTMLToken&, const QualifiedName&, const String& replacementValue = String());
     72    bool eraseAttributeIfInjected(HTMLToken&, const QualifiedName&, const String& replacementValue = String(), AttributeKind treatment = NormalAttribute);
    6873
    6974    String snippetForRange(const HTMLToken&, int start, int end);
    7075    String snippetForJavaScript(const String&);
    71     String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
     76    String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&, AttributeKind treatment = NormalAttribute);
    7277
    7378    bool isContainedInRequest(const String&);
Note: See TracChangeset for help on using the changeset viewer.