Title: [145620] branches/safari-536.30-branch

Diff

Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (145619 => 145620)


--- branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-12 23:58:51 UTC (rev 145619)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog	2013-03-13 00:00:57 UTC (rev 145620)
@@ -1,5 +1,19 @@
 2013-03-12  Lucas Forschler  <[email protected]>
 
+        Merge r135299
+
+    2012-11-20  Tom Sepez  <[email protected]>
+
+            XSSAuditor::decodedSnippetForJavaScript stopping when comma encountered.
+            https://bugs.webkit.org/show_bug.cgi?id=102587
+
+            Reviewed by Adam Barth.
+
+            * http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt: Added.
+            * http/tests/security/xssAuditor/script-tag-with-actual-comma.html: Added.
+
+2013-03-12  Lucas Forschler  <[email protected]>
+
         Merge r135193
 
     2012-11-19  Abhishek Arya  <[email protected]>

Copied: branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt (from rev 135299, trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt) (0 => 145620)


--- branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma-expected.txt	2013-03-13 00:00:57 UTC (rev 145620)
@@ -0,0 +1,4 @@
+CONSOLE MESSAGE: Refused to execute a _javascript_ script. Source code of script found within request.
+
+
+Test that the XSSAuditor's tolerance for the IIS webserver's comma concatenation doesn't open holes when the reflected argument contains an actual comma. The test passes if the XSSAuditor logs console messages and no alerts fire.

Copied: branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html (from rev 135299, trunk/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html) (0 => 145620)


--- branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html	                        (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/http/tests/security/xssAuditor/script-tag-with-actual-comma.html	2013-03-13 00:00:57 UTC (rev 145620)
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.testRunner) {
+  testRunner.dumpAsText();
+  testRunner.setXSSAuditorEnabled(true);
+}
+</script>
+</head>
+<body>
+<iframe src=""
+</iframe>
+<p>Test that the XSSAuditor's tolerance for the IIS webserver's comma concatenation doesn't open holes when the reflected argument
+contains an actual comma. The test passes if the XSSAuditor logs console messages and no alerts fire.</p>
+</body>
+</html>

Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (145619 => 145620)


--- branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-12 23:58:51 UTC (rev 145619)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog	2013-03-13 00:00:57 UTC (rev 145620)
@@ -1,5 +1,24 @@
 2013-03-12  Lucas Forschler  <[email protected]>
 
+        Merge r135299
+
+    2012-11-20  Tom Sepez  <[email protected]>
+
+            XSSAuditor::decodedSnippetForJavaScript stopping when comma encountered.
+            https://bugs.webkit.org/show_bug.cgi?id=102587
+
+            Reviewed by Adam Barth.
+
+            Rather than returning an empty fragment, continue processing the body
+            of a script tag when the decoded fragment reduces to nothing.
+
+            Test: http/tests/security/xssAuditor/script-tag-with-actual-comma.html
+
+            * html/parser/XSSAuditor.cpp:
+            (WebCore::XSSAuditor::decodedSnippetForJavaScript):
+
+2013-03-12  Lucas Forschler  <[email protected]>
+
         Merge r135193
 
     2012-11-19  Abhishek Arya  <[email protected]>

Modified: branches/safari-536.30-branch/Source/WebCore/html/parser/XSSAuditor.cpp (145619 => 145620)


--- branches/safari-536.30-branch/Source/WebCore/html/parser/XSSAuditor.cpp	2013-03-12 23:58:51 UTC (rev 145619)
+++ branches/safari-536.30-branch/Source/WebCore/html/parser/XSSAuditor.cpp	2013-03-13 00:00:57 UTC (rev 145620)
@@ -565,31 +565,35 @@
             break;
     }
 
-    // Stop at next comment (using the same rules as above for SVG/XML vs HTML), when we 
-    // encounter a comma, or when we  exceed the maximum length target. The comma rule
-    // covers a common parameter concatenation case performed by some webservers.
-    // After hitting the length target, we can only stop at a point where we know we are
-    // not in the middle of a %-escape sequence. For the sake of simplicity, approximate
-    // not stopping inside a (possibly multiply encoded) %-esacpe sequence by breaking on
-    // whitespace only. We should have enough text in these cases to avoid false positives.
-    for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
-        if (!m_shouldAllowCDATA) {
-            if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
-                endPosition = foundPosition + 2;
-                break;
+    String result;
+    while (startPosition < endPosition && !result.length()) {
+        // Stop at next comment (using the same rules as above for SVG/XML vs HTML), when we 
+        // encounter a comma, or when we  exceed the maximum length target. The comma rule
+        // covers a common parameter concatenation case performed by some webservers.
+        // After hitting the length target, we can only stop at a point where we know we are
+        // not in the middle of a %-escape sequence. For the sake of simplicity, approximate
+        // not stopping inside a (possibly multiply encoded) %-esacpe sequence by breaking on
+        // whitespace only. We should have enough text in these cases to avoid false positives.
+        for (foundPosition = startPosition; foundPosition < endPosition; foundPosition++) {
+            if (!m_shouldAllowCDATA) {
+                if (startsSingleLineCommentAt(string, foundPosition) || startsMultiLineCommentAt(string, foundPosition)) {
+                    foundPosition += 2;
+                    break;
+                }
+                if (startsHTMLCommentAt(string, foundPosition)) {
+                    foundPosition += 4;
+                    break;
+                }
             }
-            if (startsHTMLCommentAt(string, foundPosition)) {
-                endPosition = foundPosition + 4;
+            if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
                 break;
             }
         }
-        if (string[foundPosition] == ',' || (foundPosition > startPosition + kMaximumFragmentLengthTarget && isHTMLSpace(string[foundPosition]))) {
-            endPosition = foundPosition;
-            break;
-        }
+
+        result = fullyDecodeString(string.substring(startPosition, foundPosition - startPosition), m_parser->document()->decoder());
+        startPosition = foundPosition + 1;
     }
-
-    return fullyDecodeString(string.substring(startPosition, endPosition - startPosition), m_parser->document()->decoder());
+    return result;
 }
 
 bool XSSAuditor::isContainedInRequest(const String& decodedSnippet)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to