Title: [144613] trunk
- Revision
- 144613
- Author
- [email protected]
- Date
- 2013-03-04 03:25:13 -0800 (Mon, 04 Mar 2013)
Log Message
XSSAuditor should strip dangerous attributes from SMIL animation elements.
https://bugs.webkit.org/show_bug.cgi?id=111071
Reviewed by Adam Barth.
Source/WebCore:
SMIL animation elements can, amusingly enough, animate the 'href' of a
link. This patch teaches XSSAuditor how to deal with the
semicolon-separated 'animation[values]' attribute in order to ensure
that it doesn't contain _javascript_ URLs that could be animated into
place for an unsuspecting user to click on.
Test: http/tests/security/xssAuditor/svg-animate.html
* html/parser/XSSAuditor.cpp:
(WebCore::isSemicolonSeparatedAttribute): Added.
This returns true if the given attribute is SVGNames::valuesAttr,
but makes it possible to easily extend the list with additional
attributes with this strange property.
(WebCore::semicolonSeparatedValueContainsJavaScriptURL): Added.
Given a semicolon-separated string, determine if it contains any
_javascript_ URLs.
(WebCore::XSSAuditor::eraseDangerousAttributesIfInjected):
When looking for dangerousness, determine whether or not we should
be comparing against each member of a semicolon-separated list.
LayoutTests:
* http/tests/security/xssAuditor/resources/echo-intertag.pl:
Add a new mode that dumps an element and its attributes, which is a
bit more robust than adding a new mode for each attribute type we're
interested in.
* http/tests/security/xssAuditor/svg-animate-expected.txt: Added.
* http/tests/security/xssAuditor/svg-animate.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (144612 => 144613)
--- trunk/LayoutTests/ChangeLog 2013-03-04 11:09:59 UTC (rev 144612)
+++ trunk/LayoutTests/ChangeLog 2013-03-04 11:25:13 UTC (rev 144613)
@@ -1,3 +1,17 @@
+2013-03-04 Mike West <[email protected]>
+
+ XSSAuditor should strip dangerous attributes from SMIL animation elements.
+ https://bugs.webkit.org/show_bug.cgi?id=111071
+
+ Reviewed by Adam Barth.
+
+ * http/tests/security/xssAuditor/resources/echo-intertag.pl:
+ Add a new mode that dumps an element and its attributes, which is a
+ bit more robust than adding a new mode for each attribute type we're
+ interested in.
+ * http/tests/security/xssAuditor/svg-animate-expected.txt: Added.
+ * http/tests/security/xssAuditor/svg-animate.html: Added.
+
2013-03-04 Zan Dobersek <[email protected]>
Unreviewed GTK gardening.
Modified: trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl (144612 => 144613)
--- trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl 2013-03-04 11:09:59 UTC (rev 144612)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl 2013-03-04 11:25:13 UTC (rev 144613)
@@ -104,6 +104,20 @@
print " alert('formaction present on ' + e.nodeName + ' with value of ' + e.getAttribute('formaction'));\n";
print "</script>\n";
}
+if ($cgi->param('dumpElementBySelector')) {
+ print "<pre id='console'></pre>\n";
+ print "<script>\n";
+ print " var e = document.querySelector('" . $cgi->param('dumpElementBySelector') . "');\n";
+ print " if (e) {\n";
+ print " var log = '" . $cgi->param('dumpElementBySelector') . " => ' + e.nodeName + '\\n';\n";
+ print " for (var i = 0; i < e.attributes.length; i++) {\n";
+ print " log += '* ' + e.attributes[i].name + ': ' + e.attributes[i].value + '\\n';\n";
+ print " }\n";
+ print " document.getElementById('console').innerText = log;\n";
+ print " } else\n";
+ print " alert('No element matched the given selector.');\n";
+ print "</script>\n";
+}
if ($cgi->param('notifyDone')) {
print "<script>\n";
print "if (window.testRunner)\n";
Added: trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate-expected.txt (0 => 144613)
--- trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate-expected.txt 2013-03-04 11:25:13 UTC (rev 144613)
@@ -0,0 +1,16 @@
+CONSOLE MESSAGE: line 4: Refused to execute a _javascript_ script. Source code of script found within request.
+
+This test passes if the element displayed in the frame below has a 'values' attribute containing only '_javascript_:void(0)'.
+
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+animate => animate
+* attributeName: xlink:href
+* values: _javascript_:void(0)
+* begin: 0s
+* end: 0.1s
+* fill: freeze
+
Added: trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate.html (0 => 144613)
--- trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate.html (rev 0)
+++ trunk/LayoutTests/http/tests/security/xssAuditor/svg-animate.html 2013-03-04 11:25:13 UTC (rev 144613)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <script>
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.dumpChildFramesAsText();
+ testRunner.waitUntilDone();
+ testRunner.setXSSAuditorEnabled(true);
+ }
+ </script>
+</head>
+<body>
+ <p>This test passes if the element displayed in the frame below has a 'values' attribute containing only '_javascript_:void(0)'.</p>
+ <iframe src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (144612 => 144613)
--- trunk/Source/WebCore/ChangeLog 2013-03-04 11:09:59 UTC (rev 144612)
+++ trunk/Source/WebCore/ChangeLog 2013-03-04 11:25:13 UTC (rev 144613)
@@ -1,3 +1,30 @@
+2013-03-04 Mike West <[email protected]>
+
+ XSSAuditor should strip dangerous attributes from SMIL animation elements.
+ https://bugs.webkit.org/show_bug.cgi?id=111071
+
+ Reviewed by Adam Barth.
+
+ SMIL animation elements can, amusingly enough, animate the 'href' of a
+ link. This patch teaches XSSAuditor how to deal with the
+ semicolon-separated 'animation[values]' attribute in order to ensure
+ that it doesn't contain _javascript_ URLs that could be animated into
+ place for an unsuspecting user to click on.
+
+ Test: http/tests/security/xssAuditor/svg-animate.html
+
+ * html/parser/XSSAuditor.cpp:
+ (WebCore::isSemicolonSeparatedAttribute): Added.
+ This returns true if the given attribute is SVGNames::valuesAttr,
+ but makes it possible to easily extend the list with additional
+ attributes with this strange property.
+ (WebCore::semicolonSeparatedValueContainsJavaScriptURL): Added.
+ Given a semicolon-separated string, determine if it contains any
+ _javascript_ URLs.
+ (WebCore::XSSAuditor::eraseDangerousAttributesIfInjected):
+ When looking for dangerousness, determine whether or not we should
+ be comparing against each member of a semicolon-separated list.
+
2013-03-04 Arvid Nilsson <[email protected]>
[BlackBerry] New files for BlackBerry::Platform::Graphics::GraphicsContext integration
Modified: trunk/Source/WebCore/html/parser/XSSAuditor.cpp (144612 => 144613)
--- trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2013-03-04 11:09:59 UTC (rev 144612)
+++ trunk/Source/WebCore/html/parser/XSSAuditor.cpp 2013-03-04 11:25:13 UTC (rev 144613)
@@ -52,6 +52,10 @@
#include "TextResourceDecoder.h"
#include "XLinkNames.h"
+#if ENABLE(SVG)
+#include "SVGNames.h"
+#endif
+
#include <wtf/Functional.h>
#include <wtf/MainThread.h>
#include <wtf/text/CString.h>
@@ -189,6 +193,25 @@
return result;
}
+static bool isSemicolonSeparatedAttribute(const HTMLToken::Attribute& attribute)
+{
+#if ENABLE(SVG)
+ return threadSafeMatch(attribute.name, SVGNames::valuesAttr);
+#endif
+ return false;
+}
+
+static bool semicolonSeparatedValueContainsJavaScriptURL(const String& value)
+{
+ Vector<String> valueList;
+ value.split(';', valueList);
+ for (size_t i = 0; i < valueList.size(); ++i) {
+ if (protocolIsJavaScript(valueList[i]))
+ return true;
+ }
+ return false;
+}
+
XSSAuditor::XSSAuditor()
: m_isEnabled(false)
, m_xssProtection(ContentSecurityPolicy::FilterReflectedXSS)
@@ -510,7 +533,9 @@
for (size_t i = 0; i < request.token.attributes().size(); ++i) {
const HTMLToken::Attribute& attribute = request.token.attributes().at(i);
bool isInlineEventHandler = isNameOfInlineEventHandler(attribute.name);
- bool valueContainsJavaScriptURL = !isInlineEventHandler && protocolIsJavaScript(stripLeadingAndTrailingHTMLSpaces(String(attribute.value)));
+ // FIXME: It would be better if we didn't create a new String for every attribute in the document.
+ String strippedValue = stripLeadingAndTrailingHTMLSpaces(String(attribute.value));
+ bool valueContainsJavaScriptURL = (!isInlineEventHandler && protocolIsJavaScript(strippedValue)) || (isSemicolonSeparatedAttribute(attribute) && semicolonSeparatedValueContainsJavaScriptURL(strippedValue));
if (!isInlineEventHandler && !valueContainsJavaScriptURL)
continue;
if (!isContainedInRequest(decodedSnippetForAttribute(request, attribute, ScriptLikeAttribute)))
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
