Title: [94225] trunk
Revision
94225
Author
[email protected]
Date
2011-08-31 13:52:10 -0700 (Wed, 31 Aug 2011)

Log Message

Fix XSS filter bypass by multiply decoding both the URL and the body
snippet until they are in the most minimal form before comparison.
https://bugs.webkit.org/show_bug.cgi?id=66585

Patch by Tom Sepez <[email protected]> on 2011-08-31
Reviewed by Adam Barth.

Source/WebCore:

* html/parser/XSSAuditor.cpp:
(WebCore::fullyDecodeString):
(WebCore::XSSAuditor::init):
(WebCore::XSSAuditor::filterToken):
(WebCore::XSSAuditor::isContainedInRequest):

LayoutTests:

* http/tests/security/xssAuditor/anchor-url-dom-write-location2.html:
* http/tests/security/xssAuditor/resources/echo-dom-write-unescaped-location.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (94224 => 94225)


--- trunk/LayoutTests/ChangeLog	2011-08-31 20:46:22 UTC (rev 94224)
+++ trunk/LayoutTests/ChangeLog	2011-08-31 20:52:10 UTC (rev 94225)
@@ -1,3 +1,14 @@
+2011-08-31  Tom Sepez  <[email protected]>
+
+        Fix XSS filter bypass by multiply decoding both the URL and the body
+        snippet until they are in the most minimal form before comparison.
+        https://bugs.webkit.org/show_bug.cgi?id=66585
+
+        Reviewed by Adam Barth.
+
+        * http/tests/security/xssAuditor/anchor-url-dom-write-location2.html:
+        * http/tests/security/xssAuditor/resources/echo-dom-write-unescaped-location.html: Added.
+
 2011-08-31  Cary Clark  <[email protected]>
 
         Unreviewed; new baselines (Skia on Mac, next chunk of 800 files)

Modified: trunk/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location2.html (94224 => 94225)


--- trunk/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location2.html	2011-08-31 20:46:22 UTC (rev 94224)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/anchor-url-dom-write-location2.html	2011-08-31 20:52:10 UTC (rev 94225)
@@ -9,7 +9,7 @@
 </script>
 </head>
 <body>
-<iframe src=""
+<iframe src=""
 </iframe>
 </body>
 </html>

Added: trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-unescaped-location.html (0 => 94225)


--- trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-unescaped-location.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-dom-write-unescaped-location.html	2011-08-31 20:52:10 UTC (rev 94225)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<script>document.write(window.location);</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (94224 => 94225)


--- trunk/Source/WebCore/ChangeLog	2011-08-31 20:46:22 UTC (rev 94224)
+++ trunk/Source/WebCore/ChangeLog	2011-08-31 20:52:10 UTC (rev 94225)
@@ -1,3 +1,17 @@
+2011-08-31  Tom Sepez  <[email protected]>
+
+        Fix XSS filter bypass by multiply decoding both the URL and the body
+        snippet until they are in the most minimal form before comparison.
+        https://bugs.webkit.org/show_bug.cgi?id=66585
+
+        Reviewed by Adam Barth.
+
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::fullyDecodeString):
+        (WebCore::XSSAuditor::init):
+        (WebCore::XSSAuditor::filterToken):
+        (WebCore::XSSAuditor::isContainedInRequest):
+
 2011-08-31  Simon Fraser  <[email protected]>
 
         Crash with -webkit-radial-gradient(top) gradient

Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (94224 => 94225)


--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2011-08-31 20:46:22 UTC (rev 94224)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2011-08-31 20:52:10 UTC (rev 94225)
@@ -39,6 +39,7 @@
 #include "Settings.h"
 #include "TextEncoding.h"
 #include "TextResourceDecoder.h"
+
 #include <wtf/text/CString.h>
 
 namespace WebCore {
@@ -114,17 +115,23 @@
     return equalIgnoringCase(value.data() + i, _javascript_Scheme, lengthOfJavaScriptScheme);
 }
 
-static String decodeURL(const String& string, const TextEncoding& encoding)
+static String fullyDecodeString(const String& string, const TextResourceDecoder* decoder)
 {
+    size_t oldWorkingStringLength;
     String workingString = string;
+    do {
+        oldWorkingStringLength = workingString.length();
+        workingString = decodeURLEscapeSequences(workingString);
+    } while (workingString.length() < oldWorkingStringLength);
+    if (decoder) {
+        CString workingStringUTF8 = workingString.utf8();
+        String decodedString = decoder->encoding().decode(workingStringUTF8.data(), workingStringUTF8.length());
+        if (!decodedString.isEmpty())
+            workingString = decodedString;
+    }
     workingString.replace('+', ' ');
-    workingString = decodeURLEscapeSequences(workingString);
-    CString workingStringUTF8 = workingString.utf8();
-    String decodedString = encoding.decode(workingStringUTF8.data(), workingStringUTF8.length());
-    // FIXME: Is this check necessary?
-    if (decodedString.isEmpty())
-        return canonicalize(workingString);
-    return canonicalize(decodedString);
+    workingString = canonicalize(workingString);
+    return workingString;
 }
 
 XSSAuditor::XSSAuditor(HTMLDocumentParser* parser)
@@ -152,7 +159,7 @@
 
     if (!m_isEnabled)
         return;
-    
+
     // In theory, the Document could have detached from the Frame after the
     // XSSAuditor was constructed.
     if (!m_parser->document()->frame()) {
@@ -168,7 +175,7 @@
     }
 
     TextResourceDecoder* decoder = m_parser->document()->decoder();
-    m_decodedURL = decoder ? decodeURL(url.string(), decoder->encoding()) : url.string();
+    m_decodedURL = fullyDecodeString(url.string(), decoder);
     if (m_decodedURL.find(isRequiredForInjection, 0) == notFound)
         m_decodedURL = String();
 
@@ -179,7 +186,7 @@
         FormData* httpBody = documentLoader->originalRequest().httpBody();
         if (httpBody && !httpBody->isEmpty()) {
             String httpBodyAsString = httpBody->flattenToString();
-            m_decodedHTTPBody = decoder ? decodeURL(httpBodyAsString, decoder->encoding()) : httpBodyAsString;
+            m_decodedHTTPBody = fullyDecodeString(httpBodyAsString, decoder);
             if (m_decodedHTTPBody.find(isRequiredForInjection, 0) == notFound)
                 m_decodedHTTPBody = String();
             if (m_decodedHTTPBody.length() >= miniumLengthForSuffixTree)
@@ -207,7 +214,7 @@
     case Uninitialized:
         ASSERT_NOT_REACHED();
         break;
-    case Initial: 
+    case Initial:
         didBlockScript = filterTokenInitial(token);
         break;
     case AfterScriptStartTag:
@@ -454,13 +461,15 @@
 bool XSSAuditor::isContainedInRequest(const String& snippet)
 {
     ASSERT(!snippet.isEmpty());
-    String canonicalizedSnippet = canonicalize(snippet);
-    ASSERT(!canonicalizedSnippet.isEmpty());
-    if (m_decodedURL.find(canonicalizedSnippet, 0, false) != notFound)
+    TextResourceDecoder* decoder = m_parser->document()->decoder();
+    String decodedSnippet = fullyDecodeString(snippet, decoder);
+    if (decodedSnippet.isEmpty())
+        return false;
+    if (m_decodedURL.find(decodedSnippet, 0, false) != notFound)
         return true;
-    if (m_decodedHTTPBodySuffixTree && !m_decodedHTTPBodySuffixTree->mightContain(canonicalizedSnippet))
+    if (m_decodedHTTPBodySuffixTree && !m_decodedHTTPBodySuffixTree->mightContain(decodedSnippet))
         return false;
-    return m_decodedHTTPBody.find(canonicalizedSnippet, 0, false) != notFound;
+    return m_decodedHTTPBody.find(decodedSnippet, 0, false) != notFound;
 }
 
 bool XSSAuditor::isSameOriginResource(const String& url)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to