Changeset 77059 in webkit

Timestamp:

Jan 29, 2011 1:20:44 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-addslashes*
​https://bugs.webkit.org/show_bug.cgi?id=53365

We need to canonicalize strings to avoid being tricked by addslashes.

• html/parser/XSSFilter.cpp: (WebCore::HTMLNames::isNonCanonicalCharacter):
  • This function is copied from the XSSAuditor (with some tweaks). We'll eventually remove the XSSAuditor once we've got XSSFilter working properly.

(WebCore::HTMLNames::canonicalize):
(WebCore::HTMLNames::decodeURL):
(WebCore::XSSFilter::isContainedInRequest):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• html/parser/XSSFilter.cpp (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-01-29  Adam Barth  <abarth@webkit.org>
+
+        Reviewed by Daniel Bates.
+
+        XSSFilter should pass xssAuditor/script-tag-addslashes*
+        https://bugs.webkit.org/show_bug.cgi?id=53365
+
+        We need to canonicalize strings to avoid being tricked by addslashes.
+
+        * html/parser/XSSFilter.cpp:
+        (WebCore::HTMLNames::isNonCanonicalCharacter):
+            - This function is copied from the XSSAuditor (with some tweaks).
+              We'll eventually remove the XSSAuditor once we've got XSSFilter
+              working properly.
+        (WebCore::HTMLNames::canonicalize):
+        (WebCore::HTMLNames::decodeURL):
+        (WebCore::XSSFilter::isContainedInRequest):
+
 2011-01-29  Adam Barth  <abarth@webkit.org>
 

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

@@ -46 +46 @@
 namespace {
 
+bool isNonCanonicalCharacter(UChar c)
+{
+    // We remove all non-ASCII characters, including non-printable ASCII characters.
+    //
+    // 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.
+    //
+    // For instance: new String("http://localhost:8000") => new String("http://localhost:8").
+    return (c == '\\' || c == '0' || c == '\0' || c >= 127);
+}
+
+String canonicalize(const String& string)
+{
+    return string.removeCharacters(&isNonCanonicalCharacter);
+}
+
 bool hasName(const HTMLToken& token, const QualifiedName& name)
 {
@@ -79 +96 @@
     // FIXME: Is this check necessary?
     if (decodedString.isEmpty())
-        return workingString;
-    return decodedString;
+        return canonicalize(workingString);
+    return canonicalize(decodedString);
 }
 
@@ -325 +342 @@
 bool XSSFilter::isContainedInRequest(const String& snippet)
 {
-    return m_decodedURL.find(snippet, 0, false) != notFound || m_decodedHTTPBody.find(snippet, 0, false) != notFound;
+    String canonicalizedSnippet = canonicalize(snippet);
+    return m_decodedURL.find(canonicalizedSnippet, 0, false) != notFound
+        || m_decodedHTTPBody.find(canonicalizedSnippet, 0, false) != notFound;
 }