[SCM] WebKit Debian packaging branch, webkit-1.3, updated. upstream/1.3.7-4207-g178b198
abarth at webkit.org
abarth at webkit.org
Mon Feb 21 00:12:54 UTC 2011
The following commit has been merged in the webkit-1.3 branch:
commit a622eb58dd8084f045d6cc3a2aa92a645daf775e
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Jan 28 21:31:06 2011 +0000
2011-01-28 Adam Barth <abarth at webkit.org>
Reviewed by Daniel Bates.
Teach XSSFilter how to filter <script> elements
https://bugs.webkit.org/show_bug.cgi?id=53279
This patch adds the ability for the XSSFilter to block injected
<script> elements. Handling script elements is slightly subtle because
these elements act very differently depending on whether they have a
src attribute.
In the "src case", which check whether the src attribute was present in
the request. In the "non-src case", we check whether the start tag and
the body of the script element was included in the request. Checking
for the whole start tag means we miss out on some attribute splitting
attacks inside of script tags, but that doesn't seem like that big a
deal.
This patch also introduces some amount of state into the XSSFilter
because inline script elements span multiple tokens. There's a lot of
tuning and optimization left in these cases, some of which I've noted
with FIXMEs.
To test this patch, I played around with some of the existing
XSSAuditor tests. Hopefully I'll be able to run the test suite more
systematically in the future.
* html/parser/HTMLToken.h:
(WebCore::HTMLToken::eraseCharacters):
(WebCore::HTMLToken::eraseValueOfAttribute):
* html/parser/XSSFilter.cpp:
(WebCore::HTMLNames::hasName):
(WebCore::HTMLNames::findAttributeWithName):
(WebCore::HTMLNames::isNameOfScriptCarryingAttribute):
(WebCore::XSSFilter::XSSFilter):
(WebCore::XSSFilter::filterToken):
(WebCore::XSSFilter::filterTokenAfterScriptStartTag):
(WebCore::XSSFilter::filterScriptToken):
(WebCore::XSSFilter::snippetForRange):
(WebCore::XSSFilter::snippetForAttribute):
* html/parser/XSSFilter.h:
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76981 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index fb7973c..748f4c6 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -2,6 +2,49 @@
Reviewed by Daniel Bates.
+ Teach XSSFilter how to filter <script> elements
+ https://bugs.webkit.org/show_bug.cgi?id=53279
+
+ This patch adds the ability for the XSSFilter to block injected
+ <script> elements. Handling script elements is slightly subtle because
+ these elements act very differently depending on whether they have a
+ src attribute.
+
+ In the "src case", which check whether the src attribute was present in
+ the request. In the "non-src case", we check whether the start tag and
+ the body of the script element was included in the request. Checking
+ for the whole start tag means we miss out on some attribute splitting
+ attacks inside of script tags, but that doesn't seem like that big a
+ deal.
+
+ This patch also introduces some amount of state into the XSSFilter
+ because inline script elements span multiple tokens. There's a lot of
+ tuning and optimization left in these cases, some of which I've noted
+ with FIXMEs.
+
+ To test this patch, I played around with some of the existing
+ XSSAuditor tests. Hopefully I'll be able to run the test suite more
+ systematically in the future.
+
+ * html/parser/HTMLToken.h:
+ (WebCore::HTMLToken::eraseCharacters):
+ (WebCore::HTMLToken::eraseValueOfAttribute):
+ * html/parser/XSSFilter.cpp:
+ (WebCore::HTMLNames::hasName):
+ (WebCore::HTMLNames::findAttributeWithName):
+ (WebCore::HTMLNames::isNameOfScriptCarryingAttribute):
+ (WebCore::XSSFilter::XSSFilter):
+ (WebCore::XSSFilter::filterToken):
+ (WebCore::XSSFilter::filterTokenAfterScriptStartTag):
+ (WebCore::XSSFilter::filterScriptToken):
+ (WebCore::XSSFilter::snippetForRange):
+ (WebCore::XSSFilter::snippetForAttribute):
+ * html/parser/XSSFilter.h:
+
+2011-01-28 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Daniel Bates.
+
Sketch out new XSS filter design (disabled by default)
https://bugs.webkit.org/show_bug.cgi?id=53205
diff --git a/Source/WebCore/html/parser/HTMLToken.h b/Source/WebCore/html/parser/HTMLToken.h
index a2af2a4..ccbcdfd 100644
--- a/Source/WebCore/html/parser/HTMLToken.h
+++ b/Source/WebCore/html/parser/HTMLToken.h
@@ -246,6 +246,18 @@ public:
return m_data;
}
+ void eraseCharacters()
+ {
+ ASSERT(m_type == Character);
+ m_data.clear();
+ }
+
+ void eraseValueOfAttribute(size_t i)
+ {
+ ASSERT(m_type == StartTag || m_type == EndTag);
+ m_attributes[i].m_value.clear();
+ }
+
const DataVector& characters() const
{
ASSERT(m_type == Character);
diff --git a/Source/WebCore/html/parser/XSSFilter.cpp b/Source/WebCore/html/parser/XSSFilter.cpp
index e63fcb3..92ea38a 100644
--- a/Source/WebCore/html/parser/XSSFilter.cpp
+++ b/Source/WebCore/html/parser/XSSFilter.cpp
@@ -28,6 +28,7 @@
#include "Document.h"
#include "HTMLDocumentParser.h"
+#include "HTMLNames.h"
#include "TextEncoding.h"
#include "TextResourceDecoder.h"
#include <wtf/text/CString.h>
@@ -37,18 +38,32 @@
namespace WebCore {
+using namespace HTMLNames;
+
namespace {
+bool hasName(const HTMLToken& token, const QualifiedName& name)
+{
+ return equalIgnoringNullity(token.name(), static_cast<const String&>(name.localName()));
+}
+
+bool findAttributeWithName(const HTMLToken& token, const QualifiedName& name, size_t& indexOfMatchingAttribute)
+{
+ for (size_t i = 0; i < token.attributes().size(); ++i) {
+ if (equalIgnoringNullity(token.attributes().at(i).m_name, name.localName())) {
+ indexOfMatchingAttribute = i;
+ return true;
+ }
+ }
+ return false;
+}
+
bool isNameOfScriptCarryingAttribute(const Vector<UChar, 32>& name)
{
const size_t lengthOfShortestScriptCarryingAttribute = 5; // To wit: oncut.
if (name.size() < lengthOfShortestScriptCarryingAttribute)
return false;
- if (name[0] != 'o' && name[0] != 'O')
- return false;
- if (name[1] != 'n' && name[0] != 'N')
- return false;
- return true;
+ return name[0] == 'o' && name[1] == 'n';
}
String decodeURL(const String& string, const TextEncoding& encoding)
@@ -68,6 +83,7 @@ String decodeURL(const String& string, const TextEncoding& encoding)
XSSFilter::XSSFilter(HTMLDocumentParser* parser)
: m_parser(parser)
+ , m_state(Initial)
{
ASSERT(m_parser);
}
@@ -78,30 +94,87 @@ void XSSFilter::filterToken(HTMLToken& token)
ASSERT_UNUSED(token, &token);
return;
#else
+ switch (m_state) {
+ case Initial:
+ break;
+ case AfterScriptStartTag:
+ filterTokenAfterScriptStartTag(token);
+ ASSERT(m_state == Initial);
+ m_cachedSnippet = String();
+ return;
+ }
+
if (token.type() != HTMLToken::StartTag)
return;
- HTMLToken::AttributeList::const_iterator iter = token.attributes().begin();
- for (; iter != token.attributes().end(); ++iter) {
- if (!isNameOfScriptCarryingAttribute(iter->m_name))
+ if (hasName(token, scriptTag)) {
+ filterScriptToken(token);
+ return;
+ }
+
+ for (size_t i = 0; i < token.attributes().size(); ++i) {
+ const HTMLToken::Attribute& attribute = token.attributes().at(i);
+ if (!isNameOfScriptCarryingAttribute(attribute.m_name))
continue;
- if (!isContainedInRequest(snippetForAttribute(token, *iter)))
+ if (!isContainedInRequest(snippetForAttribute(token, attribute)))
continue;
- iter->m_value.clear();
+ token.eraseValueOfAttribute(i);
}
#endif
}
+void XSSFilter::filterTokenAfterScriptStartTag(HTMLToken& token)
+{
+ ASSERT(m_state == AfterScriptStartTag);
+ m_state = Initial;
+
+ if (token.type() != HTMLToken::Character) {
+ ASSERT(token.type() == HTMLToken::EndTag || token.type() == HTMLToken::EndOfFile);
+ return;
+ }
+
+ int start = 0;
+ // FIXME: We probably want to grab only the first few characters of the
+ // contents of the script element.
+ int end = token.endIndex() - token.startIndex();
+ if (isContainedInRequest(m_cachedSnippet + snippetForRange(token, start, end))) {
+ token.eraseCharacters();
+ token.appendToCharacter(' '); // Technically, character tokens can't be empty.
+ }
+}
+
+void XSSFilter::filterScriptToken(HTMLToken& token)
+{
+ ASSERT(m_state == Initial);
+ ASSERT(token.type() == HTMLToken::StartTag);
+ ASSERT(hasName(token, scriptTag));
+
+ size_t indexOfFirstSrcAttribute;
+ if (findAttributeWithName(token, srcAttr, indexOfFirstSrcAttribute)) {
+ const HTMLToken::Attribute& srcAttribute = token.attributes().at(indexOfFirstSrcAttribute);
+ if (isContainedInRequest(snippetForAttribute(token, srcAttribute)))
+ token.eraseValueOfAttribute(indexOfFirstSrcAttribute);
+ return;
+ }
+
+ m_state = AfterScriptStartTag;
+ m_cachedSnippet = m_parser->sourceForToken(token);
+}
+
+String XSSFilter::snippetForRange(const HTMLToken& token, int start, int end)
+{
+ // 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 XSSFilter::snippetForAttribute(const HTMLToken& token, const HTMLToken::Attribute& attribute)
{
// FIXME: We should grab one character before the name also.
int start = attribute.m_nameRange.m_start - token.startIndex();
// FIXME: We probably want to grab only the first few characters of the attribute value.
int end = attribute.m_valueRange.m_end - token.startIndex();
-
- // 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);
+ return snippetForRange(token, start, end);
}
bool XSSFilter::isContainedInRequest(const String& snippet)
diff --git a/Source/WebCore/html/parser/XSSFilter.h b/Source/WebCore/html/parser/XSSFilter.h
index bb2c4a4..89ac95a 100644
--- a/Source/WebCore/html/parser/XSSFilter.h
+++ b/Source/WebCore/html/parser/XSSFilter.h
@@ -40,10 +40,22 @@ public:
void filterToken(HTMLToken&);
private:
+ enum State {
+ Initial,
+ AfterScriptStartTag,
+ };
+
+ void filterTokenAfterScriptStartTag(HTMLToken&);
+ void filterScriptToken(HTMLToken&);
+
+ String snippetForRange(const HTMLToken&, int start, int end);
String snippetForAttribute(const HTMLToken&, const HTMLToken::Attribute&);
+
bool isContainedInRequest(const String&);
HTMLDocumentParser* m_parser;
+ State m_state;
+ String m_cachedSnippet;
};
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list