Changeset 49434 in webkit


Ignore:
Timestamp:
Oct 11, 2009 11:50:09 PM (15 years ago)
Author:
dbates@webkit.org
Message:

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

Reviewed by Adam Barth.

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


Fixes an issue where JavaScript URLs that are URL-encoded twice can
bypass the XSSAuditor.


JavaScript URLs that are completed by method Document::completeURL have added
URL-encoded characters such that a direct comparison with the URL-decoded
outgoing HTTP parameters is not sufficient. Instead, the URL-decoded outgoing
HTTP parameters must be URL-decoded before comparison.

Tests: http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html

http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html

  • bindings/ScriptControllerBase.cpp: (WebCore::ScriptController::executeIfJavaScriptURL): Modified to pass XSSAuditor the URL-decoded source code for the JavaScript URL.
  • page/XSSAuditor.cpp: (WebCore::isIllegalURICharacter): Minor syntactical change to the comment. (WebCore::XSSAuditor::CachingURLCanonicalizer::canonicalizeURL): Added parameter decodeURLEscapeSequencesTwice. (WebCore::XSSAuditor::canEvaluateJavaScriptURL): (WebCore::XSSAuditor::decodeURL): Ditto. (WebCore::XSSAuditor::findInRequest): Ditto.
  • page/XSSAuditor.h: (WebCore::XSSAuditor::CachingURLCanonicalizer::CachingURLCanonicalizer): Ditto.

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

Reviewed by Adam Barth.

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


Tests that JavaScript URLs that are twice URL encoded do not bypass the XSSAuditor.

  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt: Added.
  • http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html: Added.
Location:
trunk
Files:
6 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r49433 r49434  
     12009-10-11  Daniel Bates  <dbates@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=30242
     6       
     7        Tests that JavaScript URLs that are twice URL encoded do not bypass the XSSAuditor.
     8
     9        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode-expected.txt: Added.
     10        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html: Added.
     11        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2-expected.txt: Added.
     12        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html: Added.
     13        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3-expected.txt: Added.
     14        * http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html: Added.
     15
    1162009-10-11  Dan Bernstein  <mitz@apple.com>
    217

  • trunk/WebCore/ChangeLog

    r49432 r49434  
     12009-10-11  Daniel Bates  <dbates@webkit.org>
     2
     3        Reviewed by Adam Barth.
     4
     5        https://bugs.webkit.org/show_bug.cgi?id=30242
     6       
     7        Fixes an issue where JavaScript URLs that are URL-encoded twice can
     8        bypass the XSSAuditor.
     9       
     10        JavaScript URLs that are completed by method Document::completeURL have added
     11        URL-encoded characters such that a direct comparison with the URL-decoded
     12        outgoing HTTP parameters is not sufficient. Instead, the URL-decoded outgoing
     13        HTTP parameters must be URL-decoded before comparison.
     14
     15        Tests: http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode.html
     16               http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode2.html
     17               http/tests/security/xssAuditor/iframe-javascript-url-twice-url-encode3.html
     18
     19        * bindings/ScriptControllerBase.cpp:
     20        (WebCore::ScriptController::executeIfJavaScriptURL): Modified to pass XSSAuditor
     21        the URL-decoded source code for the JavaScript URL.
     22        * page/XSSAuditor.cpp:
     23        (WebCore::isIllegalURICharacter): Minor syntactical change to the comment.
     24        (WebCore::XSSAuditor::CachingURLCanonicalizer::canonicalizeURL): Added
     25        parameter decodeURLEscapeSequencesTwice.
     26        (WebCore::XSSAuditor::canEvaluateJavaScriptURL):
     27        (WebCore::XSSAuditor::decodeURL): Ditto.
     28        (WebCore::XSSAuditor::findInRequest): Ditto.
     29        * page/XSSAuditor.h:
     30        (WebCore::XSSAuditor::CachingURLCanonicalizer::CachingURLCanonicalizer): Ditto.
     31
    1322009-10-11  Dominic Cooney  <dominicc@google.com>
    233

  • trunk/WebCore/bindings/ScriptControllerBase.cpp

    r49372 r49434  
    6464    const int javascriptSchemeLength = sizeof("javascript:") - 1;
    6565
    66     String script = url.string().substring(javascriptSchemeLength);
     66    String script = decodeURLEscapeSequences(url.string().substring(javascriptSchemeLength));
    6767    ScriptValue result;
    6868    if (xssAuditor()->canEvaluateJavaScriptURL(script))
    69         result = executeScript(decodeURLEscapeSequences(script), userGesture);
     69        result = executeScript(script, userGesture);
    7070
    7171    String scriptResult;

  • trunk/WebCore/page/XSSAuditor.cpp

    r48961 r49434  
    6666    //
    6767    // If the request does not contain these characters then we can assume that no inline scripts have been injected
    68     // into response page, because it is impossible to write an inline script of the form <script>...</script>
     68    // into the response page, because it is impossible to write an inline script of the form <script>...</script>
    6969    // without "<", ">".
    7070    return (c == '\'' || c == '"' || c == '<' || c == '>');
    7171}
    7272
    73 String XSSAuditor::CachingURLCanonicalizer::canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities)
    74 {
    75     if (decodeEntities == m_decodeEntities && encoding == m_encoding && url == m_inputURL)
     73String XSSAuditor::CachingURLCanonicalizer::canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
     74                                                            bool decodeURLEscapeSequencesTwice)
     75{
     76    if (decodeEntities == m_decodeEntities && decodeURLEscapeSequencesTwice == m_decodeURLEscapeSequencesTwice
     77        && encoding == m_encoding && url == m_inputURL)
    7678        return m_cachedCanonicalizedURL;
    7779
    78     m_cachedCanonicalizedURL = canonicalize(decodeURL(url, encoding, decodeEntities));
     80    m_cachedCanonicalizedURL = canonicalize(decodeURL(url, encoding, decodeEntities, decodeURLEscapeSequencesTwice));
    7981    m_inputURL = url;
    8082    m_encoding = encoding;
    8183    m_decodeEntities = decodeEntities;
     84    m_decodeURLEscapeSequencesTwice = decodeURLEscapeSequencesTwice;
    8285    return m_cachedCanonicalizedURL;
    8386}
    … …  
    116119        return true;
    117120
    118     if (findInRequest(code)) {
     121    if (findInRequest(code, true, false, true)) {
    119122        DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
    120123        m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
    … …  
    183186}
    184187
    185 String XSSAuditor::decodeURL(const String& string, const TextEncoding& encoding, bool decodeEntities)
     188String XSSAuditor::decodeURL(const String& string, const TextEncoding& encoding, bool decodeEntities, bool decodeURLEscapeSequencesTwice)
    186189{
    187190    String result;
    … …  
    194197    if (!decodedResult.isEmpty())
    195198        result = decodedResult;
     199    if (decodeURLEscapeSequencesTwice) {
     200        result = decodeURLEscapeSequences(result);
     201        utf8Url = result.utf8();
     202        decodedResult = encoding.decode(utf8Url.data(), utf8Url.length());
     203        if (!decodedResult.isEmpty())
     204            result = decodedResult;
     205    }
    196206    if (decodeEntities)
    197207        result = decodeHTMLEntities(result);
    … …  
    236246}
    237247
    238 bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters) const
     248bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
     249                               bool decodeURLEscapeSequencesTwice) const
    239250{
    240251    bool result = false;
    241252    Frame* parentFrame = m_frame->tree()->parent();
    242253    if (parentFrame && m_frame->document()->url() == blankURL())
    243         result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters);
     254        result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
    244255    if (!result)
    245         result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters);
     256        result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
    246257    return result;
    247258}
    248259
    249 bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters) const
     260bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters,
     261                               bool decodeURLEscapeSequencesTwice) const
    250262{
    251263    ASSERT(frame->document());
    … …  
    286298    if (string.length() < pageURL.length()) {
    287299        // The string can actually fit inside the pageURL.
    288         String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities);
     300        String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
    289301
    290302        if (allowRequestIfNoIllegalURICharacters && (!formDataObj || formDataObj->isEmpty())
    … …  
    303315            // code is less than or equal to the length of the url-encoded
    304316            // string.
    305             String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities);
     317            String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
    306318            if (decodedFormData.find(canonicalizedString, 0, false) != -1)
    307319                return true;  // We found the string in the POST data.

  • trunk/WebCore/page/XSSAuditor.h

    r48961 r49434  
    103103        {
    104104        public:
    105             CachingURLCanonicalizer() : m_decodeEntities(false) { }
    106             String canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities);
     105            CachingURLCanonicalizer() : m_decodeEntities(false), m_decodeURLEscapeSequencesTwice(false) { }
     106            String canonicalizeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
     107                                   bool decodeURLEscapeSequencesTwice);
    107108
    108109        private:
    … …  
    111112            TextEncoding m_encoding;
    112113            bool m_decodeEntities;
     114            bool m_decodeURLEscapeSequencesTwice;
    113115
    114116            // The cached result.
    … …  
    117119
    118120        static String canonicalize(const String&);
    119         static String decodeURL(const String& url, const TextEncoding& encoding, bool decodeEntities);
     121        static String decodeURL(const String& url, const TextEncoding& encoding, bool decodeEntities,
     122                                bool decodeURLEscapeSequencesTwice = false);
    120123        static String decodeHTMLEntities(const String&, bool leaveUndecodableEntitiesUntouched = true);
    121124
    122         bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false) const;
    123         bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false) const;
     125        bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
     126                           bool decodeURLEscapeSequencesTwice = false) const;
     127        bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false,
     128                           bool decodeURLEscapeSequencesTwice = false) const;
    124129
    125130        // The frame to audit.
Note: See TracChangeset for help on using the changeset viewer.