XSSAuditor should strip formaction attributes from input and button elements.
authormkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Feb 2013 12:22:11 +0000 (12:22 +0000)
committermkwst@chromium.org <mkwst@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Feb 2013 12:22:11 +0000 (12:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=110975

Reviewed by Daniel Bates.

Source/WebCore:

The 'formaction' attribute of 'input' and 'button' elements is just as
dangerous as the 'action' attribute of 'form' elements. This patch
teaches the XSSAuditor how to avoid them.

Tests: http/tests/security/xssAuditor/formaction-on-button.html
       http/tests/security/xssAuditor/formaction-on-input.html

* html/parser/XSSAuditor.cpp:
(WebCore::XSSAuditor::filterStartToken):
(WebCore::XSSAuditor::filterInputToken): Added.
(WebCore::XSSAuditor::filterButtonToken): Added.
* html/parser/XSSAuditor.h:
    Create filters for 'input' and 'button' elements, which currently
    only have the effect of filtering the 'formaction' attribute.

LayoutTests:

* http/tests/security/xssAuditor/formaction-on-button-expected.txt: Added.
* http/tests/security/xssAuditor/formaction-on-button.html: Added.
* http/tests/security/xssAuditor/formaction-on-input-expected.txt: Added.
* http/tests/security/xssAuditor/formaction-on-input.html: Added.
* http/tests/security/xssAuditor/resources/echo-intertag.pl:
    Support 'showFormaction' as a new option to write out formaction values.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@144292 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/security/xssAuditor/formaction-on-button-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/formaction-on-button.html [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/formaction-on-input-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/formaction-on-input.html [new file with mode: 0644]
LayoutTests/http/tests/security/xssAuditor/resources/echo-intertag.pl
Source/WebCore/ChangeLog
Source/WebCore/html/parser/XSSAuditor.cpp
Source/WebCore/html/parser/XSSAuditor.h

index eeae1ac..fb2bd15 100644 (file)
@@ -1,3 +1,17 @@
+2013-02-28  Mike West  <mkwst@chromium.org>
+
+        XSSAuditor should strip formaction attributes from input and button elements.
+        https://bugs.webkit.org/show_bug.cgi?id=110975
+
+        Reviewed by Daniel Bates.
+
+        * http/tests/security/xssAuditor/formaction-on-button-expected.txt: Added.
+        * http/tests/security/xssAuditor/formaction-on-button.html: Added.
+        * http/tests/security/xssAuditor/formaction-on-input-expected.txt: Added.
+        * http/tests/security/xssAuditor/formaction-on-input.html: Added.
+        * http/tests/security/xssAuditor/resources/echo-intertag.pl:
+            Support 'showFormaction' as a new option to write out formaction values.
+
 2013-02-28  Takashi Toyoshima  <toyoshim@chromium.org>
 
         Unreviewed gardening, clean up expectations to remove lint errors #1.
diff --git a/LayoutTests/http/tests/security/xssAuditor/formaction-on-button-expected.txt b/LayoutTests/http/tests/security/xssAuditor/formaction-on-button-expected.txt
new file mode 100644 (file)
index 0000000..fd01e91
--- /dev/null
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: line 4: Refused to execute a JavaScript script. Source code of script found within request.
+
+ALERT: formaction present on BUTTON with value of about:blank
+
diff --git a/LayoutTests/http/tests/security/xssAuditor/formaction-on-button.html b/LayoutTests/http/tests/security/xssAuditor/formaction-on-button.html
new file mode 100644 (file)
index 0000000..f13e384
--- /dev/null
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+            testRunner.setXSSAuditorEnabled(true);
+        }
+    </script>
+</head>
+<body>
+    <iframe src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<form><button%20formaction='http://example.com/'>&notifyDone=1&showFormaction=1"></iframe>
+</body>
+</html>
diff --git a/LayoutTests/http/tests/security/xssAuditor/formaction-on-input-expected.txt b/LayoutTests/http/tests/security/xssAuditor/formaction-on-input-expected.txt
new file mode 100644 (file)
index 0000000..e0ed30c
--- /dev/null
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: line 4: Refused to execute a JavaScript script. Source code of script found within request.
+
+ALERT: formaction present on INPUT with value of about:blank
+
diff --git a/LayoutTests/http/tests/security/xssAuditor/formaction-on-input.html b/LayoutTests/http/tests/security/xssAuditor/formaction-on-input.html
new file mode 100644 (file)
index 0000000..760139d
--- /dev/null
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <script>
+        if (window.testRunner) {
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+            testRunner.setXSSAuditorEnabled(true);
+        }
+    </script>
+</head>
+<body>
+    <iframe src="http://localhost:8000/security/xssAuditor/resources/echo-intertag.pl?q=<form><input%20formaction='http://example.com/'>&notifyDone=1&showFormaction=1"></iframe>
+</body>
+</html>
+
index 93ece45..c6139b4 100755 (executable)
@@ -97,6 +97,13 @@ if ($cgi->param('showAction')) {
     print "    alert('Form action set to ' + document.forms[0].action);\n";
     print "</script>\n";
 }
+if ($cgi->param('showFormaction')) {
+    print "<script>\n";
+    print "    var e = document.querySelector('[formaction]');\n";
+    print "    if (e)\n";
+    print "        alert('formaction present on ' + e.nodeName + ' with value of ' + e.getAttribute('formaction'));\n";
+    print "</script>\n";
+}
 if ($cgi->param('notifyDone')) {
     print "<script>\n";
     print "if (window.testRunner)\n";
index c8f3fb3..fd192f4 100644 (file)
@@ -1,3 +1,25 @@
+2013-02-28  Mike West  <mkwst@chromium.org>
+
+        XSSAuditor should strip formaction attributes from input and button elements.
+        https://bugs.webkit.org/show_bug.cgi?id=110975
+
+        Reviewed by Daniel Bates.
+
+        The 'formaction' attribute of 'input' and 'button' elements is just as
+        dangerous as the 'action' attribute of 'form' elements. This patch
+        teaches the XSSAuditor how to avoid them.
+
+        Tests: http/tests/security/xssAuditor/formaction-on-button.html
+               http/tests/security/xssAuditor/formaction-on-input.html
+
+        * html/parser/XSSAuditor.cpp:
+        (WebCore::XSSAuditor::filterStartToken):
+        (WebCore::XSSAuditor::filterInputToken): Added.
+        (WebCore::XSSAuditor::filterButtonToken): Added.
+        * html/parser/XSSAuditor.h:
+            Create filters for 'input' and 'button' elements, which currently
+            only have the effect of filtering the 'formaction' attribute.
+
 2013-02-28  Allan Sandfeld Jensen  <allan.jensen@digia.com>
 
         REGRESSION(r144169): It broke clipping
index b6b346d..7848c26 100644 (file)
@@ -343,6 +343,10 @@ bool XSSAuditor::filterStartToken(const FilterTokenRequest& request)
         didBlockScript |= filterBaseToken(request);
     else if (hasName(request.token, formTag))
         didBlockScript |= filterFormToken(request);
+    else if (hasName(request.token, inputTag))
+        didBlockScript |= filterInputToken(request);
+    else if (hasName(request.token, buttonTag))
+        didBlockScript |= filterButtonToken(request);
 
     return didBlockScript;
 }
@@ -477,6 +481,22 @@ bool XSSAuditor::filterFormToken(const FilterTokenRequest& request)
     return eraseAttributeIfInjected(request, actionAttr, blankURL().string());
 }
 
+bool XSSAuditor::filterInputToken(const FilterTokenRequest& request)
+{
+    ASSERT(request.token.type() == HTMLToken::StartTag);
+    ASSERT(hasName(request.token, inputTag));
+
+    return eraseAttributeIfInjected(request, formactionAttr, blankURL().string(), SrcLikeAttribute);
+}
+
+bool XSSAuditor::filterButtonToken(const FilterTokenRequest& request)
+{
+    ASSERT(request.token.type() == HTMLToken::StartTag);
+    ASSERT(hasName(request.token, buttonTag));
+
+    return eraseAttributeIfInjected(request, formactionAttr, blankURL().string(), SrcLikeAttribute);
+}
+
 bool XSSAuditor::eraseDangerousAttributesIfInjected(const FilterTokenRequest& request)
 {
     DEFINE_STATIC_LOCAL(String, safeJavaScriptURL, (ASCIILiteral("javascript:void(0)")));
index 6262505..4cf8301 100644 (file)
@@ -87,6 +87,8 @@ private:
     bool filterMetaToken(const FilterTokenRequest&);
     bool filterBaseToken(const FilterTokenRequest&);
     bool filterFormToken(const FilterTokenRequest&);
+    bool filterInputToken(const FilterTokenRequest&);
+    bool filterButtonToken(const FilterTokenRequest&);
 
     bool eraseDangerousAttributesIfInjected(const FilterTokenRequest&);
     bool eraseAttributeIfInjected(const FilterTokenRequest&, const QualifiedName&, const String& replacementValue = String(), AttributeKind treatment = NormalAttribute);