Merge 108881 - XSS Auditor targeting legitimate frames as false positives.

Source/WebCore/html/parser/XSSAuditor.cpp

 /*
  * Copyright (C) 2011 Adam Barth. All Rights Reserved.
  * Copyright (C) 2011 Daniel Bates (dbates@intudata.com).
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 259 matching lines @@
     switch (m_state) {
     case Uninitialized:
         ASSERT_NOT_REACHED();
         break;
     case Initial:
         didBlockScript = filterTokenInitial(token);
         break;
     case AfterScriptStartTag:
         didBlockScript = filterTokenAfterScriptStartTag(token);
         ASSERT(m_state == Initial);
-        m_cachedSnippet = String();
+        m_cachedDecodedSnippet = String();
         break;
     }
 
     if (didBlockScript) {
         // FIXME: Consider using a more helpful console message.
         DEFINE_STATIC_LOCAL(String, consoleMessage, ("Refused to execute a JavaScript script. Source code of script found within request.\n"));
         m_parser->document()->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, consoleMessage);
 
         bool didBlockEntirePage = (m_xssProtection == XSSProtectionBlockEnabled);
         if (didBlockEntirePage)
@@ skipping 43 matching lines @@
 bool XSSAuditor::filterTokenAfterScriptStartTag(HTMLToken& token)
 {
     ASSERT(m_state == AfterScriptStartTag);
     m_state = Initial;
 
     if (token.type() != HTMLTokenTypes::Character) {
         ASSERT(token.type() == HTMLTokenTypes::EndTag || token.type() == HTMLTokenTypes::EndOfFile);
         return false;
     }
 
-    TextResourceDecoder* decoder = m_parser->document()->decoder();
-    if (isContainedInRequest(fullyDecodeString(m_cachedSnippet, decoder))) {
-        int start = 0;
-        int end = token.endIndex() - token.startIndex();
-        String snippet = snippetForJavaScript(snippetForRange(token, start, end));
-        if (isContainedInRequest(fullyDecodeString(snippet, decoder))) {
-            token.eraseCharacters();
-            token.appendToCharacter(' '); // Technically, character tokens can't be empty.
-            return true;
-        }
+    if (isContainedInRequest(m_cachedDecodedSnippet) && isContainedInRequest(decodedSnippetForJavaScript(token))) {
+        token.eraseCharacters();
+        token.appendToCharacter(' '); // Technically, character tokens can't be empty.
+        return true;
     }
     return false;
 }
 
 bool XSSAuditor::filterScriptToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, scriptTag));
 
-    if (eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute))
-        return true;
+    m_state = AfterScriptStartTag;
+    m_cachedDecodedSnippet = stripLeadingAndTrailingHTMLSpaces(decodedSnippetForToken(token));
 
-    m_state = AfterScriptStartTag;
-    m_cachedSnippet = m_parser->sourceForToken(token);
+    if (isContainedInRequest(decodedSnippetForName(token)))
+        return eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute);
+
     return false;
 }
 
 bool XSSAuditor::filterObjectToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, objectTag));
 
     bool didBlockScript = false;
-
-    didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string(), SrcLikeAttribute);
-    didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
-    didBlockScript |= eraseAttributeIfInjected(token, classidAttr);
-
+    if (isContainedInRequest(decodedSnippetForName(token))) {
+        didBlockScript |= eraseAttributeIfInjected(token, dataAttr, blankURL().string(), SrcLikeAttribute);
+        didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
+        didBlockScript |= eraseAttributeIfInjected(token, classidAttr);
+    }
     return didBlockScript;
 }
 
 bool XSSAuditor::filterParamToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, paramTag));
 
     size_t indexOfNameAttribute;
     if (!findAttributeWithName(token, nameAttr, indexOfNameAttribute))
         return false;
 
     const HTMLToken::Attribute& nameAttribute = token.attributes().at(indexOfNameAttribute);
     String name = String(nameAttribute.m_value.data(), nameAttribute.m_value.size());
 
     if (!HTMLParamElement::isURLParameter(name))
         return false;
 
     return eraseAttributeIfInjected(token, valueAttr, blankURL().string(), SrcLikeAttribute);
 }
 
 bool XSSAuditor::filterEmbedToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, embedTag));
 
     bool didBlockScript = false;
-
-    didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
-    didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute);
-    didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
-
+    if (isContainedInRequest(decodedSnippetForName(token))) {
+        didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
+        didBlockScript |= eraseAttributeIfInjected(token, srcAttr, blankURL().string(), SrcLikeAttribute);
+        didBlockScript |= eraseAttributeIfInjected(token, typeAttr);
+    }
     return didBlockScript;
 }
 
 bool XSSAuditor::filterAppletToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, appletTag));
 
     bool didBlockScript = false;
-
-    didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
-    didBlockScript |= eraseAttributeIfInjected(token, objectAttr);
-
+    if (isContainedInRequest(decodedSnippetForName(token))) {
+        didBlockScript |= eraseAttributeIfInjected(token, codeAttr, String(), SrcLikeAttribute);
+        didBlockScript |= eraseAttributeIfInjected(token, objectAttr);
+    }
     return didBlockScript;
 }
 
 bool XSSAuditor::filterIframeToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, iframeTag));
 
-    return eraseAttributeIfInjected(token, srcAttr, String(), SrcLikeAttribute);
+    if (isContainedInRequest(decodedSnippetForName(token)))
+        return eraseAttributeIfInjected(token, srcAttr, String(), SrcLikeAttribute);
+
+    return false;
 }
 
 bool XSSAuditor::filterMetaToken(HTMLToken& token)
 {
     ASSERT(m_state == Initial);
     ASSERT(token.type() == HTMLTokenTypes::StartTag);
     ASSERT(hasName(token, metaTag));
 
     return eraseAttributeIfInjected(token, http_equivAttr);
 }
@@ skipping 72 matching lines @@
                 return false;
             token.eraseValueOfAttribute(indexOfAttribute);
             if (!replacementValue.isEmpty())
                 token.appendToAttributeValue(indexOfAttribute, replacementValue);
             return true;
         }
     }
     return false;
 }
 
-String XSSAuditor::snippetForRange(const HTMLToken& token, int start, int end)
+String XSSAuditor::decodedSnippetForToken(const HTMLToken& token)
 {
-    // FIXME: There's an extra allocation here that we could save by
-    //        passing the range to the parser.
-    return m_parser->sourceForToken(token).substring(start, end - start);
+    String snippet = m_parser->sourceForToken(token);
+    return fullyDecodeString(snippet, m_parser->document()->decoder());
+}
+
+String XSSAuditor::decodedSnippetForName(const HTMLToken& token)
+{
+    // Grab a fixed number of characters equal to the length of the token's
+    // name plus one (to account for the "<").
+    return decodedSnippetForToken(token).substring(0, token.name().size() + 1);
 }
 
 String XSSAuditor::decodedSnippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute, AttributeKind treatment)
 {
     const size_t kMaximumSnippetLength = 100;
 
     // The range doesn't inlcude the character which terminates the value. So,
     // for an input of |name="value"|, the snippet is |name="value|. For an
     // unquoted input of |name=value |, the snippet is |name=value|.
     // FIXME: We should grab one character before the name also.
     int start = attribute.m_nameRange.m_start - token.startIndex();
     int end = attribute.m_valueRange.m_end - token.startIndex();
-    String decodedSnippet = fullyDecodeString(snippetForRange(token, start, end), m_parser->document()->decoder());
+    String decodedSnippet = fullyDecodeString(m_parser->sourceForToken(token).substring(start, end - start), 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;
 }
 
-bool XSSAuditor::isContainedInRequest(const String& decodedSnippet)
+String XSSAuditor::decodedSnippetForJavaScript(const HTMLToken& token)
 {
-    if (decodedSnippet.isEmpty())
-        return false;
-    if (m_decodedURL.find(decodedSnippet, 0, false) != notFound)
-        return true;
-    if (m_decodedHTTPBodySuffixTree && !m_decodedHTTPBodySuffixTree->mightContain(decodedSnippet))
-        return false;
-    return m_decodedHTTPBody.find(decodedSnippet, 0, false) != notFound;
-}
-
-bool XSSAuditor::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());
-}
-
-String XSSAuditor::snippetForJavaScript(const String& string)
-{
+    String string = m_parser->sourceForToken(token);
     const size_t kMaximumFragmentLengthTarget = 100;
 
     size_t startPosition = 0;
     size_t endPosition = string.length();
     size_t foundPosition = notFound;
 
     // Skip over initial comments to find start of code.
     while (startPosition < endPosition) {
         while (startPosition < endPosition && isHTMLSpace(string[startPosition]))
             startPosition++;
@@ skipping 25 matching lines @@
         if (startsHTMLCommentAt(string, foundPosition)) {
             endPosition = foundPosition + 4;
             break;
         }
         if (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition])) {
             endPosition = foundPosition;
             break;
         }
     }
     
-    return string.substring(startPosition, endPosition - startPosition);
+    return fullyDecodeString(string.substring(startPosition, endPosition - startPosition), m_parser->document()->decoder());
+}
+
+bool XSSAuditor::isContainedInRequest(const String& decodedSnippet)
+{
+    if (decodedSnippet.isEmpty())
+        return false;
+    if (m_decodedURL.find(decodedSnippet, 0, false) != notFound)
+        return true;
+    if (m_decodedHTTPBodySuffixTree && !m_decodedHTTPBodySuffixTree->mightContain(decodedSnippet))
+        return false;
+    return m_decodedHTTPBody.find(decodedSnippet, 0, false) != notFound;
+}
+
+bool XSSAuditor::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());
 }
 
 } // namespace WebCore