Changeset 195073 in webkit


Ignore:
Timestamp:
Jan 14, 2016 1:37:49 PM (8 years ago)
Author:
dbates@webkit.org
Message:

[XSS Auditor] Partial bypass when web server collapses path components
https://bugs.webkit.org/show_bug.cgi?id=152872

Reviewed by Brent Fulgham.

Merged from Blink (patch by Tom Sepez <tsepez@chromium.org>):
<https://src.chromium.org/viewvc/blink?revision=167610&view=revision>

Source/WebCore:

Test: http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html

  • html/parser/XSSAuditor.cpp:

(WebCore::isNonCanonicalCharacter):
(WebCore::XSSAuditor::init):
(WebCore::XSSAuditor::decodedSnippetForName):
(WebCore::XSSAuditor::decodedSnippetForAttribute):
(WebCore::XSSAuditor::decodedSnippetForJavaScript):
(WebCore::fullyDecodeString): Deleted.

LayoutTests:

  • http/tests/security/xssAuditor/embed-tag-in-path-unterminated-expected.txt: Added.
  • http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html: Added.
  • http/tests/security/xssAuditor/intercept/.htaccess:
Location:
trunk
Files:
2 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r195071 r195073  
     12016-01-14  Daniel Bates  <dabates@apple.com>
     2
     3        [XSS Auditor] Partial bypass when web server collapses path components
     4        https://bugs.webkit.org/show_bug.cgi?id=152872
     5
     6        Reviewed by Brent Fulgham.
     7
     8        Merged from Blink (patch by Tom Sepez <tsepez@chromium.org>):
     9        <https://src.chromium.org/viewvc/blink?revision=167610&view=revision>
     10
     11        * http/tests/security/xssAuditor/embed-tag-in-path-unterminated-expected.txt: Added.
     12        * http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html: Added.
     13        * http/tests/security/xssAuditor/intercept/.htaccess:
     14
    1152016-01-14  Zalan Bujtas  <zalan@apple.com>
    216
  • trunk/LayoutTests/http/tests/security/xssAuditor/intercept/.htaccess

    r194978 r195073  
     1# For ease in testing path reflections, pass any path component containing <'"
     2# (and subsequent characters) as the "q" query parameter to the script identified
     3# by the path components preceeding it.
    14RewriteEngine on
    2 RewriteRule ^(.*)/(.*) /security/xssAuditor/resources/$1?q=$2 [L,NS]
     5RewriteRule ^([^<"']*)/(.*) /security/xssAuditor/resources/$1?q=$2 [L,NS]
  • trunk/Source/WebCore/ChangeLog

    r195072 r195073  
     12016-01-14  Daniel Bates  <dabates@apple.com>
     2
     3        [XSS Auditor] Partial bypass when web server collapses path components
     4        https://bugs.webkit.org/show_bug.cgi?id=152872
     5
     6        Reviewed by Brent Fulgham.
     7
     8        Merged from Blink (patch by Tom Sepez <tsepez@chromium.org>):
     9        <https://src.chromium.org/viewvc/blink?revision=167610&view=revision>
     10
     11        Test: http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html
     12
     13        * html/parser/XSSAuditor.cpp:
     14        (WebCore::isNonCanonicalCharacter):
     15        (WebCore::XSSAuditor::init):
     16        (WebCore::XSSAuditor::decodedSnippetForName):
     17        (WebCore::XSSAuditor::decodedSnippetForAttribute):
     18        (WebCore::XSSAuditor::decodedSnippetForJavaScript):
     19        (WebCore::fullyDecodeString): Deleted.
     20
    1212016-01-14  Beth Dakin  <bdakin@apple.com>
    222
  • trunk/Source/WebCore/html/parser/XSSAuditor.cpp

    r194982 r195073  
    5757    // Instead, we remove backslashes and zeros (since the string "\\0" =(remove backslashes)=> "0"). However, this has the
    5858    // adverse effect that we remove any legitimate zeros from a string.
     59    // We also remove forward-slash, because it is common for some servers to collapse successive path components, eg,
     60    // a//b becomes a/b.
    5961    //
    60     // For instance: new String("http://localhost:8000") => new String("http://localhost:8").
    61     return (c == '\\' || c == '0' || c == '\0' || c >= 127);
     62    // For instance: new String("http://localhost:8000") => new String("http:localhost:8").
     63    return (c == '\\' || c == '0' || c == '\0' || c == '/' || c >= 127);
    6264}
    6365
     
    176178    } while (workingString.length() < oldWorkingStringLength);
    177179    workingString.replace('+', ' ');
    178     workingString = canonicalize(workingString);
    179180    return workingString;
    180181}
     
    269270        m_encoding = document->decoder()->encoding();
    270271
    271     m_decodedURL = fullyDecodeString(m_documentURL.string(), m_encoding);
     272    m_decodedURL = canonicalize(fullyDecodeString(m_documentURL.string(), m_encoding));
    272273    if (m_decodedURL.find(isRequiredForInjection) == notFound)
    273274        m_decodedURL = String();
     
    307308            httpBodyAsString = httpBody->flattenToString();
    308309            if (!httpBodyAsString.isEmpty()) {
    309                 m_decodedHTTPBody = fullyDecodeString(httpBodyAsString, m_encoding);
     310                m_decodedHTTPBody = canonicalize(fullyDecodeString(httpBodyAsString, m_encoding));
    310311                if (m_decodedHTTPBody.find(isRequiredForInjection) == notFound)
    311312                    m_decodedHTTPBody = String();
     
    568569{
    569570    // Grab a fixed number of characters equal to the length of the token's name plus one (to account for the "<").
    570     return fullyDecodeString(request.sourceTracker.source(request.token), m_encoding).substring(0, request.token.name().size() + 1);
     571    return canonicalize(fullyDecodeString(request.sourceTracker.source(request.token), m_encoding).substring(0, request.token.name().size() + 1));
    571572}
    572573
     
    579580    unsigned start = attribute.startOffset;
    580581    unsigned end = attribute.endOffset;
     582
     583    // We defer canonicalizing the decoded string here to preserve embedded slashes (if any) that
     584    // may lead us to truncate the string.
    581585    String decodedSnippet = fullyDecodeString(request.sourceTracker.source(request.token, start, end), m_encoding);
    582586    decodedSnippet.truncate(kMaximumFragmentLengthTarget);
     
    627631        }
    628632    }
    629     return decodedSnippet;
     633    return canonicalize(decodedSnippet);
    630634}
    631635
     
    698702        }
    699703
    700         result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding);
     704        result = canonicalize(fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding));
    701705        startPosition = foundPosition + 1;
    702706    }
Note: See TracChangeset for help on using the changeset viewer.