[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373

dbates at webkit.org dbates at webkit.org
Thu Apr 8 00:49:12 UTC 2010


The following commit has been merged in the webkit-1.2 branch:
commit 31b52a6a857910c5898d6f622f400b12695779b0
Author: dbates at webkit.org <dbates at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Dec 25 23:32:31 2009 +0000

    2009-12-25  Daniel Bates  <dbates at webkit.org>
    
            Reviewed by Adam Barth.
    
            https://bugs.webkit.org/show_bug.cgi?id=32938
    
            Cleans up XSSAuditor.
    
            Currently, we pass various parameters through to XSSAuditor::findInRequest
            that are used to determine how to decode the HTTP input parameters so that
            we can perform a match against the script source. Instead, we have defined
            a structure XSSAuditor::FindTask that can hold all of these parameters.
    
            No functionality was changed. So, no new tests.
    
            * page/XSSAuditor.cpp:
            (WebCore::XSSAuditor::canEvaluate): Modified to use struct
            XSSAuditor::FindTask.
            (WebCore::XSSAuditor::canEvaluateJavaScriptURL): Ditto.
            (WebCore::XSSAuditor::canCreateInlineEventListener): Ditto.
            (WebCore::XSSAuditor::canLoadExternalScriptFromSrc): Ditto.
            (WebCore::XSSAuditor::canLoadObject): Ditto.
            (WebCore::XSSAuditor::canSetBaseElementURL): Ditto.
            (WebCore::XSSAuditor::findInRequest): Ditto.
            * page/XSSAuditor.h:
            (WebCore::XSSAuditor::FindTask::FindTask): Added.
    
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@52561 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 060d741..5dda417 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2009-12-25  Daniel Bates  <dbates at webkit.org>
+
+        Reviewed by Adam Barth.
+
+        https://bugs.webkit.org/show_bug.cgi?id=32938
+
+        Cleans up XSSAuditor.
+
+        Currently, we pass various parameters through to XSSAuditor::findInRequest
+        that are used to determine how to decode the HTTP input parameters so that
+        we can perform a match against the script source. Instead, we have defined
+        a structure XSSAuditor::FindTask that can hold all of these parameters.
+
+        No functionality was changed. So, no new tests.
+
+        * page/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::canEvaluate): Modified to use struct
+        XSSAuditor::FindTask.
+        (WebCore::XSSAuditor::canEvaluateJavaScriptURL): Ditto.
+        (WebCore::XSSAuditor::canCreateInlineEventListener): Ditto.
+        (WebCore::XSSAuditor::canLoadExternalScriptFromSrc): Ditto.
+        (WebCore::XSSAuditor::canLoadObject): Ditto.
+        (WebCore::XSSAuditor::canSetBaseElementURL): Ditto.
+        (WebCore::XSSAuditor::findInRequest): Ditto.
+        * page/XSSAuditor.h:
+        (WebCore::XSSAuditor::FindTask::FindTask): Added.
+
 2009-12-25  Nikolas Zimmermann  <nzimmermann at rim.com>
 
         Reviewed by Dirk Schulze.
diff --git a/WebCore/page/XSSAuditor.cpp b/WebCore/page/XSSAuditor.cpp
index 4845145..4753cbc 100644
--- a/WebCore/page/XSSAuditor.cpp
+++ b/WebCore/page/XSSAuditor.cpp
@@ -105,7 +105,12 @@ bool XSSAuditor::canEvaluate(const String& code) const
     if (!isEnabled())
         return true;
 
-    if (findInRequest(code, false, true)) {
+    FindTask task;
+    task.string = code;
+    task.decodeEntities = false;
+    task.allowRequestIfNoIllegalURICharacters = true;
+
+    if (findInRequest(task)) {
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -118,7 +123,11 @@ bool XSSAuditor::canEvaluateJavaScriptURL(const String& code) const
     if (!isEnabled())
         return true;
 
-    if (findInRequest(code, true, false, true)) {
+    FindTask task;
+    task.string = code;
+    task.decodeURLEscapeSequencesTwice = true;
+
+    if (findInRequest(task)) {
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -131,7 +140,11 @@ bool XSSAuditor::canCreateInlineEventListener(const String&, const String& code)
     if (!isEnabled())
         return true;
 
-    if (findInRequest(code, true, true)) {
+    FindTask task;
+    task.string = code;
+    task.allowRequestIfNoIllegalURICharacters = true;
+
+    if (findInRequest(task)) {
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -147,7 +160,11 @@ bool XSSAuditor::canLoadExternalScriptFromSrc(const String& context, const Strin
     if (isSameOriginResource(url))
         return true;
 
-    if (findInRequest(context + url)) {
+    FindTask task;
+    task.context = context;
+    task.string = url;
+
+    if (findInRequest(task)) {
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -163,7 +180,11 @@ bool XSSAuditor::canLoadObject(const String& url) const
     if (isSameOriginResource(url))
         return true;
 
-    if (findInRequest(url, true, true)) {
+    FindTask task;
+    task.string = url;
+    task.allowRequestIfNoIllegalURICharacters = true;
+
+    if (findInRequest(task)) {
         String consoleMessage = String::format("Refused to load an object. URL found within request: \"%s\".\n", url.utf8().data());
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -179,7 +200,10 @@ bool XSSAuditor::canSetBaseElementURL(const String& url) const
     if (isSameOriginResource(url))
         return true;
 
-    if (findInRequest(url)) {
+    FindTask task;
+    task.string = url;
+
+    if (findInRequest(task)) {
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to load from document base URL. URL found within request.\n"));
         m_frame->domWindow()->console()->addMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage, 1, String());
         return false;
@@ -265,20 +289,18 @@ bool XSSAuditor::isSameOriginResource(const String& url) const
     return (m_frame->document()->url().host() == resourceURL.host() && resourceURL.query().isEmpty());
 }
 
-bool XSSAuditor::findInRequest(const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters, 
-                               bool decodeURLEscapeSequencesTwice) const
+bool XSSAuditor::findInRequest(const FindTask& task) const
 {
     bool result = false;
     Frame* parentFrame = m_frame->tree()->parent();
     if (parentFrame && m_frame->document()->url() == blankURL())
-        result = findInRequest(parentFrame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
+        result = findInRequest(parentFrame, task);
     if (!result)
-        result = findInRequest(m_frame, string, decodeEntities, allowRequestIfNoIllegalURICharacters, decodeURLEscapeSequencesTwice);
+        result = findInRequest(m_frame, task);
     return result;
 }
 
-bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEntities, bool allowRequestIfNoIllegalURICharacters, 
-                               bool decodeURLEscapeSequencesTwice) const
+bool XSSAuditor::findInRequest(Frame* frame, const FindTask& task) const
 {
     ASSERT(frame->document());
 
@@ -287,17 +309,19 @@ bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEn
         return false;
     }
 
-    if (string.isEmpty())
+    if (task.string.isEmpty())
         return false;
 
     FormData* formDataObj = frame->loader()->documentLoader()->originalRequest().httpBody();
+    const bool hasFormData = formDataObj && !formDataObj->isEmpty();
     String pageURL = frame->document()->url().string();
 
-    if (!formDataObj && string.length() >= 2 * pageURL.length()) {
+    String canonicalizedString;
+    if (!hasFormData && task.string.length() > 2 * pageURL.length()) {
         // Q: Why do we bother to do this check at all?
         // A: Canonicalizing large inline scripts can be expensive.  We want to
-        //    bail out before the call to canonicalize below, which could
-        //    result in an unneeded allocation and memcpy.
+        //    reduce the size of the string before we call canonicalize below,
+        //    since it could result in an unneeded allocation and memcpy.
         //
         // Q: Why do we multiply by two here?
         // A: We attempt to detect reflected XSS even when the server
@@ -305,39 +329,32 @@ bool XSSAuditor::findInRequest(Frame* frame, const String& string, bool decodeEn
         //    attacker can do get the server to inflate his/her input by a
         //    factor of two by sending " characters, which the server
         //    transforms to \".
-        return false;
-    }
+        canonicalizedString = task.string.substring(0, 2 * pageURL.length());
+    } else
+        canonicalizedString = task.string;
 
     if (frame->document()->url().protocolIs("data"))
         return false;
 
-    String canonicalizedString = canonicalize(string);
+    canonicalizedString = canonicalize(canonicalizedString);
     if (canonicalizedString.isEmpty())
         return false;
 
-    if (string.length() < pageURL.length()) {
-        // The string can actually fit inside the pageURL.
-        String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
+    if (!task.context.isEmpty())
+        canonicalizedString = task.context + canonicalizedString;
 
-        if (allowRequestIfNoIllegalURICharacters && (!formDataObj || formDataObj->isEmpty()) 
-            && decodedPageURL.find(&isIllegalURICharacter, 0) == -1)
-            return false; // Injection is impossible because the request does not contain any illegal URI characters. 
+    String decodedPageURL = m_cache.canonicalizeURL(pageURL, frame->document()->decoder()->encoding(), task.decodeEntities, task.decodeURLEscapeSequencesTwice);
 
-        if (decodedPageURL.find(canonicalizedString, 0, false) != -1)
-            return true;  // We've found the smoking gun.
-    }
+    if (task.allowRequestIfNoIllegalURICharacters && !hasFormData && decodedPageURL.find(&isIllegalURICharacter, 0) == -1)
+        return false; // Injection is impossible because the request does not contain any illegal URI characters.
 
-    if (formDataObj && !formDataObj->isEmpty()) {
-        String formData = formDataObj->flattenToString();
-        if (string.length() < formData.length()) {
-            // Notice it is sufficient to compare the length of the string to
-            // the url-encoded POST data because the length of the url-decoded
-            // code is less than or equal to the length of the url-encoded
-            // string.
-            String decodedFormData = m_cache.canonicalizeURL(formData, frame->document()->decoder()->encoding(), decodeEntities, decodeURLEscapeSequencesTwice);
-            if (decodedFormData.find(canonicalizedString, 0, false) != -1)
-                return true;  // We found the string in the POST data.
-        }
+    if (decodedPageURL.find(canonicalizedString, 0, false) != -1)
+        return true; // We've found the string in the GET data.
+
+    if (hasFormData) {
+        String decodedFormData = m_cache.canonicalizeURL(formDataObj->flattenToString(), frame->document()->decoder()->encoding(), task.decodeEntities, task.decodeURLEscapeSequencesTwice);
+        if (decodedFormData.find(canonicalizedString, 0, false) != -1)
+            return true; // We found the string in the POST data.
     }
 
     return false;
diff --git a/WebCore/page/XSSAuditor.h b/WebCore/page/XSSAuditor.h
index b64665b..d976f52 100644
--- a/WebCore/page/XSSAuditor.h
+++ b/WebCore/page/XSSAuditor.h
@@ -120,16 +120,29 @@ namespace WebCore {
             String m_cachedCanonicalizedURL;
         };
 
+        struct FindTask {
+            FindTask()
+                : decodeEntities(true)
+                , allowRequestIfNoIllegalURICharacters(false)
+                , decodeURLEscapeSequencesTwice(false)
+            {
+            }
+
+            String context;
+            String string;
+            bool decodeEntities;
+            bool allowRequestIfNoIllegalURICharacters;
+            bool decodeURLEscapeSequencesTwice;
+        };
+
         static String canonicalize(const String&);
         static String decodeURL(const String& url, const TextEncoding& encoding, bool decodeEntities, 
                                 bool decodeURLEscapeSequencesTwice = false);
         static String decodeHTMLEntities(const String&, bool leaveUndecodableEntitiesUntouched = true);
 
         bool isSameOriginResource(const String& url) const;
-        bool findInRequest(const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false, 
-                           bool decodeURLEscapeSequencesTwice = false) const;
-        bool findInRequest(Frame*, const String&, bool decodeEntities = true, bool allowRequestIfNoIllegalURICharacters = false, 
-                           bool decodeURLEscapeSequencesTwice = false) const;
+        bool findInRequest(const FindTask&) const;
+        bool findInRequest(Frame*, const FindTask&) const;
 
         // The frame to audit.
         Frame* m_frame;

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list