[XSS Auditor] Partial bypass when web server collapses path components
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jan 2016 21:37:49 +0000 (21:37 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 14 Jan 2016 21:37:49 +0000 (21:37 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@195073 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/intercept/.htaccess
Source/WebCore/ChangeLog
Source/WebCore/html/parser/XSSAuditor.cpp

index 4080b81..f6bcf0f 100644 (file)
@@ -1,3 +1,17 @@
+2016-01-14  Daniel Bates  <dabates@apple.com>
+
+        [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>
+
+        * 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:
+
 2016-01-14  Zalan Bujtas  <zalan@apple.com>
 
         [iOS Simulator] fast/table/003.html failing
diff --git a/LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated-expected.txt b/LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated-expected.txt
new file mode 100644 (file)
index 0000000..6dfa338
--- /dev/null
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: line 4: The XSS Auditor refused to execute a script in 'http://localhost:8000/security/xssAuditor/intercept/echo-intertag.pl/%3Cembed%20height=%22500%22src=%22https://127.0.0.1:8443/security/xssAuditor/resources/dummy.swf%22.xml&clutter=%3Cp%3E' because its source code was found within the request. The auditor was enabled as the server sent neither an 'X-XSS-Protection' nor 'Content-Security-Policy' header.
+Check that the XSSAuditor catches reflected tags in path components
+
+
diff --git a/LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html b/LayoutTests/http/tests/security/xssAuditor/embed-tag-in-path-unterminated.html
new file mode 100644 (file)
index 0000000..9f404fe
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<p>Check that the XSSAuditor catches reflected tags in path components</p>
+<iframe src="http://localhost:8000/security/xssAuditor/intercept/echo-intertag.pl/<embed height=%22500%22src=%22https://127.0.0.1:8443/security/xssAuditor/resources/dummy.swf%22.xml&clutter=<p>"></iframe>
+</body>
+</html>
index 8770630..baba3d8 100644 (file)
@@ -1,2 +1,5 @@
+# For ease in testing path reflections, pass any path component containing <'"
+# (and subsequent characters) as the "q" query parameter to the script identified
+# by the path components preceeding it.
 RewriteEngine on
-RewriteRule ^(.*)/(.*) /security/xssAuditor/resources/$1?q=$2 [L,NS]
+RewriteRule ^([^<"']*)/(.*) /security/xssAuditor/resources/$1?q=$2 [L,NS]
index 84f8284..1b5a43f 100644 (file)
@@ -1,3 +1,23 @@
+2016-01-14  Daniel Bates  <dabates@apple.com>
+
+        [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>
+
+        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.
+
 2016-01-14  Beth Dakin  <bdakin@apple.com>
 
         imported/blink/editing/text-iterator/read-past-cloned-first-letter.html 
index 06a9bd0..38c5208 100644 (file)
@@ -56,9 +56,11 @@ static bool isNonCanonicalCharacter(UChar c)
     // Note, we don't remove backslashes like PHP stripslashes(), which among other things converts "\\0" to the \0 character.
     // Instead, we remove backslashes and zeros (since the string "\\0" =(remove backslashes)=> "0"). However, this has the
     // adverse effect that we remove any legitimate zeros from a string.
+    // We also remove forward-slash, because it is common for some servers to collapse successive path components, eg,
+    // a//b becomes a/b.
     //
-    // For instance: new String("http://localhost:8000") => new String("http://localhost:8").
-    return (c == '\\' || c == '0' || c == '\0' || c >= 127);
+    // For instance: new String("http://localhost:8000") => new String("http:localhost:8").
+    return (c == '\\' || c == '0' || c == '\0' || c == '/' || c >= 127);
 }
 
 static String canonicalize(const String& string)
@@ -175,7 +177,6 @@ static String fullyDecodeString(const String& string, const TextEncoding& encodi
         workingString = decode16BitUnicodeEscapeSequences(decodeStandardURLEscapeSequences(workingString, encoding));
     } while (workingString.length() < oldWorkingStringLength);
     workingString.replace('+', ' ');
-    workingString = canonicalize(workingString);
     return workingString;
 }
 
@@ -268,7 +269,7 @@ void XSSAuditor::init(Document* document, XSSAuditorDelegate* auditorDelegate)
     if (document->decoder())
         m_encoding = document->decoder()->encoding();
 
-    m_decodedURL = fullyDecodeString(m_documentURL.string(), m_encoding);
+    m_decodedURL = canonicalize(fullyDecodeString(m_documentURL.string(), m_encoding));
     if (m_decodedURL.find(isRequiredForInjection) == notFound)
         m_decodedURL = String();
 
@@ -306,7 +307,7 @@ void XSSAuditor::init(Document* document, XSSAuditorDelegate* auditorDelegate)
         if (httpBody && !httpBody->isEmpty()) {
             httpBodyAsString = httpBody->flattenToString();
             if (!httpBodyAsString.isEmpty()) {
-                m_decodedHTTPBody = fullyDecodeString(httpBodyAsString, m_encoding);
+                m_decodedHTTPBody = canonicalize(fullyDecodeString(httpBodyAsString, m_encoding));
                 if (m_decodedHTTPBody.find(isRequiredForInjection) == notFound)
                     m_decodedHTTPBody = String();
                 if (m_decodedHTTPBody.length() >= minimumLengthForSuffixTree)
@@ -567,7 +568,7 @@ bool XSSAuditor::eraseAttributeIfInjected(const FilterTokenRequest& request, con
 String XSSAuditor::decodedSnippetForName(const FilterTokenRequest& request)
 {
     // Grab a fixed number of characters equal to the length of the token's name plus one (to account for the "<").
-    return fullyDecodeString(request.sourceTracker.source(request.token), m_encoding).substring(0, request.token.name().size() + 1);
+    return canonicalize(fullyDecodeString(request.sourceTracker.source(request.token), m_encoding).substring(0, request.token.name().size() + 1));
 }
 
 String XSSAuditor::decodedSnippetForAttribute(const FilterTokenRequest& request, const HTMLToken::Attribute& attribute, AttributeKind treatment)
@@ -578,6 +579,9 @@ String XSSAuditor::decodedSnippetForAttribute(const FilterTokenRequest& request,
     // FIXME: We should grab one character before the name also.
     unsigned start = attribute.startOffset;
     unsigned end = attribute.endOffset;
+
+    // We defer canonicalizing the decoded string here to preserve embedded slashes (if any) that
+    // may lead us to truncate the string.
     String decodedSnippet = fullyDecodeString(request.sourceTracker.source(request.token, start, end), m_encoding);
     decodedSnippet.truncate(kMaximumFragmentLengthTarget);
     if (treatment == SrcLikeAttribute) {
@@ -626,7 +630,7 @@ String XSSAuditor::decodedSnippetForAttribute(const FilterTokenRequest& request,
             decodedSnippet.truncate(position);
         }
     }
-    return decodedSnippet;
+    return canonicalize(decodedSnippet);
 }
 
 String XSSAuditor::decodedSnippetForJavaScript(const FilterTokenRequest& request)
@@ -697,7 +701,7 @@ String XSSAuditor::decodedSnippetForJavaScript(const FilterTokenRequest& request
                 lastNonSpacePosition = foundPosition;
         }
 
-        result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding);
+        result = canonicalize(fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_encoding));
         startPosition = foundPosition + 1;
     }
     return result;