[webkit-changes] [97715] trunk

Mon, 17 Oct 2011 21:36:25 -0700

Title: [97715] trunk
Revision
97715
Author
[email protected]
Date
2011-10-17 21:37:22 -0700 (Mon, 17 Oct 2011)

Log Message

XSSAuditor bypass with remote script ending in ? character
https://bugs.webkit.org/show_bug.cgi?id=70255

Patch by Tom Sepez <[email protected]> on 2011-10-17
Reviewed by Daniel Bates.

Fix XSSAuditor bypass where unterminated src="" attribute could pick up
text from page causing failed XSS detection.  Constrain match to domain
portions of src attribute only.

Source/WebCore:

Test: http/tests/security/xssAuditor/script-tag-with-source-unterminated.html

* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::filterScriptToken):
(WebCore::XSSAuditor::filterObjectToken):
(WebCore::XSSAuditor::filterParamToken):
(WebCore::XSSAuditor::filterEmbedToken):
(WebCore::XSSAuditor::filterAppletToken):
(WebCore::XSSAuditor::filterIframeToken):
(WebCore::XSSAuditor::eraseAttributeIfInjected):
(WebCore::XSSAuditor::decodedSnippetForAttribute):
* html/parser/XSSAuditor.h:

LayoutTests:

* http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt: Added.
* http/tests/security/xssAuditor/script-tag-with-source-unterminated.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (97714 => 97715)


--- trunk/LayoutTests/ChangeLog	2011-10-18 04:31:00 UTC (rev 97714)
+++ trunk/LayoutTests/ChangeLog	2011-10-18 04:37:22 UTC (rev 97715)
@@ -1,3 +1,17 @@
+2011-10-17  Tom Sepez  <[email protected]>
+
+        XSSAuditor bypass with remote script ending in ? character
+        https://bugs.webkit.org/show_bug.cgi?id=70255
+
+        Reviewed by Daniel Bates.
+
+        Fix XSSAuditor bypass where unterminated src="" attribute could pick up
+        text from page causing failed XSS detection.  Constrain match to domain
+        portions of src attribute only.
+
+        * http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt: Added.
+        * http/tests/security/xssAuditor/script-tag-with-source-unterminated.html: Added.
+
 2011-10-17  Kent Tamura  <[email protected]>
 
         [Chromium] Test expectation update

Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt (0 => 97715)


--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated-expected.txt	2011-10-18 04:37:22 UTC (rev 97715)
@@ -0,0 +1,7 @@
+CONSOLE MESSAGE: line 1: Refused to execute a _javascript_ script. Source code of script found within request.
+
+CONSOLE MESSAGE: line 1: Refused to execute a _javascript_ script. Source code of script found within request.
+
+CONSOLE MESSAGE: line 1: Refused to execute a _javascript_ script. Source code of script found within request.
+
+

Added: trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated.html (0 => 97715)


--- trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated.html	                        (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-source-unterminated.html	2011-10-18 04:37:22 UTC (rev 97715)
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+  layoutTestController.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=''>
+</iframe>
+<iframe src=''>
+</iframe>
+<iframe src=''>
+</iframe>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (97714 => 97715)


--- trunk/Source/WebCore/ChangeLog	2011-10-18 04:31:00 UTC (rev 97714)
+++ trunk/Source/WebCore/ChangeLog	2011-10-18 04:37:22 UTC (rev 97715)
@@ -1,3 +1,27 @@
+2011-10-17  Tom Sepez  <[email protected]>
+
+        XSSAuditor bypass with remote script ending in ? character
+        https://bugs.webkit.org/show_bug.cgi?id=70255
+
+        Reviewed by Daniel Bates.
+
+        Fix XSSAuditor bypass where unterminated src="" attribute could pick up
+        text from page causing failed XSS detection.  Constrain match to domain
+        portions of src attribute only.
+
+        Test: http/tests/security/xssAuditor/script-tag-with-source-unterminated.html
+
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::filterScriptToken):
+        (WebCore::XSSAuditor::filterObjectToken):
+        (WebCore::XSSAuditor::filterParamToken):
+        (WebCore::XSSAuditor::filterEmbedToken):
+        (WebCore::XSSAuditor::filterAppletToken):
+        (WebCore::XSSAuditor::filterIframeToken):
+        (WebCore::XSSAuditor::eraseAttributeIfInjected):
+        (WebCore::XSSAuditor::decodedSnippetForAttribute):
+        * html/parser/XSSAuditor.h:
+
 2011-10-17  Adam Klein  <[email protected]>
 
         Parse MutationObserverOptions directly into a bitfield

Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (97714 => 97715)


--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2011-10-18 04:31:00 UTC (rev 97714)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp	2011-10-18 04:37:22 UTC (rev 97715)
@@ -353,7 +353,7 @@
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, scriptTag));
 
-    if (eraseAttributeIfInjected(token, srcAttr, blankURL().string()))
+    if (eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute))
         return true;
 
     m_state = AfterScriptStartTag;
@@ -369,7 +369,7 @@
 
     bool didBlockScript = false;
 
-    didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string());
+    didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string(), SrcLikeAttribute);
     didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
     didBlockScript |= eraseAttributeIfInjected(token, classidAttr);
 
@@ -392,7 +392,7 @@
     if (!HTMLParamElement::isURLParameter(name))
         return false;
 
-    return eraseAttributeIfInjected(token, valueAttr, blankURL().string());
+    return eraseAttributeIfInjected(token, valueAttr, blankURL().string(), SrcLikeAttribute);
 }
 
 bool XSSAuditor::filterEmbedToken(HTMLToken& token)
@@ -403,7 +403,7 @@
 
     bool didBlockScript = false;
 
-    didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string());
+    didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute);
     didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
 
     return didBlockScript;
@@ -417,7 +417,7 @@
 
     bool didBlockScript = false;
 
-    didBlockScript |= eraseAttributeIfInjected(token, codeAttr);
+    didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
     didBlockScript |= eraseAttributeIfInjected(token, objectAttr);
 
     return didBlockScript;
@@ -429,7 +429,7 @@
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, iframeTag));
 
-    return eraseAttributeIfInjected(token, srcAttr);
+    return eraseAttributeIfInjected(token, srcAttr, String(), SrcLikeAttribute);
 }
 
 bool XSSAuditor::filterMetaToken(HTMLToken& token)
@@ -502,12 +502,12 @@
     return didBlockScript;
 }
 
-bool XSSAuditor::eraseAttributeIfInjected(HTMLToken& token, const QualifiedName& attributeName, const String& replacementValue)
+bool XSSAuditor::eraseAttributeIfInjected(HTMLToken& token, const QualifiedName& attributeName, const String& replacementValue, AttributeKind treatment)
 {
     size_t indexOfAttribute;
     if (findAttributeWithName(token, attributeName, indexOfAttribute)) {
         const HTMLToken::Attribute& attribute = token.attributes().at(indexOfAttribute);
-        if (isContainedInRequest(decodedSnippetForAttribute(token, attribute))) {
+        if (isContainedInRequest(decodedSnippetForAttribute(token, attribute, treatment))) {
             if (attributeName == srcAttr && isSameOriginResource(String(attribute.m_value.data(), attribute.m_value.size())))
                 return false;
             if (attributeName == http_equivAttr && !isDangerousHTTPEquiv(String(attribute.m_value.data(), attribute.m_value.size())))
@@ -528,7 +528,7 @@
     return m_parser->sourceForToken(token).substring(start, end - start);
 }
 
-String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
+String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute, AttributeKind treatment)
 {
     const size_t kMaximumSnippetLength = 100;
 
@@ -540,6 +540,20 @@
     int end = attribute.m_valueRange.m_end - token.startIndex();
     String decodedSnippet = fullyDecodeString(snippetForRange(token, start, end), m_parser->document()->decoder());
     decodedSnippet.truncate(kMaximumSnippetLength);
+    if (treatment == SrcLikeAttribute) {
+        int slashCount;
+        size_t currentLength;
+        // Characters following the first ?, #, or third slash may come from 
+        // the page itself and can be merely ignored by an attacker's server
+        // when a remote script or script-like resource is requested.
+        for (slashCount = 0, currentLength = 0; currentLength < decodedSnippet.length(); ++currentLength) {
+            if (decodedSnippet[currentLength] == '?' || decodedSnippet[currentLength] == '#'
+                || ((decodedSnippet[currentLength] == '/' || decodedSnippet[currentLength] == '\\') && ++slashCount > 2)) {
+                decodedSnippet.truncate(currentLength);
+                break;
+            }
+        }
+    }
     return decodedSnippet;
 }
 
@@ -566,7 +580,6 @@
     return (m_parser->document()->url().host() == resourceURL.host() && resourceURL.query().isEmpty());
 }
 
-
 String XSSAuditor::snippetForJavaScript(const String& string)
 {
     const size_t kMaximumFragmentLengthTarget = 100;

Modified: trunk/Source/WebCore/html/parser/XSSAuditor.h (97714 => 97715)


--- trunk/Source/WebCore/html/parser/XSSAuditor.h	2011-10-18 04:31:00 UTC (rev 97714)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.h	2011-10-18 04:37:22 UTC (rev 97715)
@@ -48,6 +48,11 @@
         AfterScriptStartTag,
     };
 
+    enum AttributeKind {
+        NormalAttribute,
+        SrcLikeAttribute
+    };
+
     void init();
 
     bool filterTokenInitial(HTMLToken&);
@@ -64,11 +69,11 @@
     bool filterFormToken(HTMLToken&);
 
     bool eraseDangerousAttributesIfInjected(HTMLToken&);
-    bool eraseAttributeIfInjected(HTMLToken&, const QualifiedName&, const String& replacementValue = String());
+    bool eraseAttributeIfInjected(HTMLToken&, const QualifiedName&, const String& replacementValue = String(), AttributeKind treatment = NormalAttribute);
 
     String snippetForRange(const HTMLToken&, int start, int end);
     String snippetForJavaScript(const String&);
-    String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
+    String decodedSnippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&, AttributeKind treatment = NormalAttribute);
 
     bool isContainedInRequest(const String&);
     bool isSameOriginResource(const String& url);

_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to