Changeset 77058 in webkit

Timestamp:

Jan 29, 2011 1:19:21 AM (13 years ago)

Author:

abarth@webkit.org

Message:

2011-01-29 Adam Barth <​abarth@webkit.org>

Reviewed by Daniel Bates.

XSSFilter should pass xssAuditor/script-tag-with-source-same-host.html
and xssAuditor/script-tag-post-*
​https://bugs.webkit.org/show_bug.cgi?id=53364

We're supposed to allow loading same-origin resources even if they
appear as part of the request.

Also, we're supposed to look at the POST data too. :)

• html/parser/XSSFilter.cpp: (WebCore::XSSFilter::eraseAttributeIfInjected): (WebCore::XSSFilter::isSameOriginResource):
  • Copy/paste from XSSAuditor::isSameOriginResource. We'll eventually remove the XSSAuditor version when XSSFilter is done.
• html/parser/XSSFilter.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/XSSFilter.cpp (modified) (5 diffs)
• html/parser/XSSFilter.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-01-29  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Daniel Bates.
+
+        XSSFilter should pass xssAuditor/script-tag-with-source-same-host.html
+        and xssAuditor/script-tag-post-*
+        https://bugs.webkit.org/show_bug.cgi?id=53364
+
+        We're supposed to allow loading same-origin resources even if they
+        appear as part of the request.
+
+        Also, we're supposed to look at the POST data too.  :)
+
+        * html/parser/XSSFilter.cpp:
+        (WebCore::XSSFilter::eraseAttributeIfInjected):
+        (WebCore::XSSFilter::isSameOriginResource):
+            - Copy/paste from XSSAuditor::isSameOriginResource.  We'll
+              eventually remove the XSSAuditor version when XSSFilter is done.
+        * html/parser/XSSFilter.h:
+
 2011-01-29  Adam Barth  <abarth@webkit.org>
 

trunk/Source/WebCore/html/parser/XSSFilter.cpp

@@ -28 +28 @@
 
 #include "Document.h"
+#include "DocumentLoader.h"
 #include "Frame.h"
 #include "HTMLDocumentParser.h"
@@ -94 +95 @@
             m_isEnabled = settings->xssAuditorEnabled();
     }
+    // Although tempting to call init() at this point, the various objects
+    // we want to reference might not all have been constructed yet.
+}
+
+void XSSFilter::init()
+{
+    ASSERT(m_isEnabled);
+
+    const TextEncoding& encoding = m_parser->document()->decoder()->encoding();
+    String url = m_parser->document()->url().string();
+    m_decodedURL = decodeURL(url, encoding);
+
+    // In theory, the Document could have detached from the Frame after the
+    // XSSFilter was constructed.
+    if (!m_parser->document()->frame())
+        return;
+
+    if (DocumentLoader* documentLoader = m_parser->document()->frame()->loader()->documentLoader()) {
+        FormData* httpBody = documentLoader->originalRequest().httpBody();
+        if (httpBody && !httpBody->isEmpty())
+            m_decodedHTTPBody = decodeURL(httpBody->flattenToString(), encoding);
+    }
 }
 
@@ -104 +127 @@
     if (!m_isEnabled)
         return;
+
+    if (m_decodedURL.isEmpty())
+        init();
 
     bool didBlockScript = false;
@@ -270 +296 @@
         const HTMLToken::Attribute& attribute = token.attributes().at(indexOfAttribute);
         if (isContainedInRequest(snippetForAttribute(token, attribute))) {
+            if (attributeName == srcAttr && isSameOriginResource(String(attribute.m_value.data(), attribute.m_value.size())))
+                return false;
             token.eraseValueOfAttribute(indexOfAttribute);
             if (!replacementValue.isEmpty())
@@ -297 +325 @@
 bool XSSFilter::isContainedInRequest(const String& snippet)
 {
-    String url = m_parser->document()->url().string();
-    String decodedURL = decodeURL(url, m_parser->document()->decoder()->encoding());
-    if (decodedURL.find(snippet, 0, false) != notFound)
-        return true; // We've found the string in the GET data.
-    // FIXME: Look in form data.
-    return false;
-}
-
-}
+    return m_decodedURL.find(snippet, 0, false) != notFound || m_decodedHTTPBody.find(snippet, 0, false) != notFound;
+}
+
+bool XSSFilter::isSameOriginResource(const String& url)
+{
+    // If the resource is loaded from the same URL as the enclosing page, it's
+    // probably not an XSS attack, so we reduce false positives by allowing the
+    // request. If the resource has a query string, we're more suspicious,
+    // however, because that's pretty rare and the attacker might be able to
+    // trick a server-side script into doing something dangerous with the query
+    // string.
+    KURL resourceURL(m_parser->document()->url(), url);
+    return (m_parser->document()->url().host() == resourceURL.host() && resourceURL.query().isEmpty());
+}
+
+}

trunk/Source/WebCore/html/parser/XSSFilter.h

@@ -46 +46 @@
     };
 
+    void init();
+
     bool filterTokenInitial(HTMLToken&);
     bool filterTokenAfterScriptStartTag(HTMLToken&);
@@ -63 +65 @@
 
     bool isContainedInRequest(const String&);
+    bool isSameOriginResource(const String& url);
 
     HTMLDocumentParser* m_parser;
     bool m_isEnabled;
+
+    String m_decodedURL;
+    String m_decodedHTTPBody;
+
     State m_state;
     String m_cachedSnippet;