Title: [210077] trunk
Revision
210077
Author
[email protected]
Date
2016-12-21 14:06:22 -0800 (Wed, 21 Dec 2016)

Log Message

Switch to a blacklist model for restricted Accept headers in simple CORS requests
https://bugs.webkit.org/show_bug.cgi?id=166363

Reviewed by Alex Christensen.

Source/WebCore:

Updated existing tests.

* platform/network/HTTPParsers.cpp:
(WebCore::isDelimiterCharacter):
    Convenience function for checking delimiter characters according to:
    https://tools.ietf.org/html/rfc7230#section-3.2.6
(WebCore::isValidAcceptHeaderValue):
    Now uses WebCore::isDelimiterCharacter() to blacklist delimiter characters
    instead of a whitelist of accepted non-alphanumeric characters.

LayoutTests:

* http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt:
* http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (210076 => 210077)


--- trunk/LayoutTests/ChangeLog	2016-12-21 22:02:48 UTC (rev 210076)
+++ trunk/LayoutTests/ChangeLog	2016-12-21 22:06:22 UTC (rev 210077)
@@ -1,3 +1,13 @@
+2016-12-21  John Wilander  <[email protected]>
+
+        Switch to a blacklist model for restricted Accept headers in simple CORS requests
+        https://bugs.webkit.org/show_bug.cgi?id=166363
+
+        Reviewed by Alex Christensen.
+
+        * http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt:
+        * http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html:
+
 2016-12-21  Wenson Hsieh  <[email protected]>
 
         Add a layout test for scroll snapping with padding in the container

Modified: trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt (210076 => 210077)


--- trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt	2016-12-21 22:02:48 UTC (rev 210076)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt	2016-12-21 22:06:22 UTC (rev 210077)
@@ -3,11 +3,29 @@
 CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Content-Language is not allowed by Access-Control-Allow-Headers.
 CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Content-Language is not allowed by Access-Control-Allow-Headers.
 CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
-PASS Accept header with normal value SHOULD NOT cause a preflight
-PASS Accept header value with all allowed non-alphanumeric characters SHOULD NOT cause a preflight
-PASS Accept-Language header with normal value SHOULD NOT cause a preflight
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest/resources/cors-preflight-safelisted-headers-responder.php. Request header field Accept is not allowed by Access-Control-Allow-Headers.
+PASS Accept header value 'application/json,text/*,*/*' SHOULD NOT cause a preflight
+PASS Accept header with normal value 'application/vnd.api+json' SHOULD NOT cause a preflight
+PASS Accept header with normal value 'text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c' SHOULD NOT cause a preflight
+PASS Accept header with normal value 'text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5' SHOULD NOT cause a preflight
+PASS Accept header value with all allowed delimiter characters SHOULD NOT cause a preflight
+PASS Accept-Language header value 'en-US,en;q=0.8' SHOULD NOT cause a preflight
+PASS Accept-Language header value 'zh-Latn-CN-variant1-a-extend1-x-wadegile-private1' SHOULD NOT cause a preflight
 PASS Accept-Language header value with all allowed non-alphanumeric characters SHOULD NOT cause a preflight
-PASS Content-Language header with normal value SHOULD NOT cause a preflight
+PASS Content-Language header value 'en-US,en;q=0.8' SHOULD NOT cause a preflight
+PASS Content-Language header value 'zh-Latn-CN-variant1-a-extend1-x-wadegile-private1' SHOULD NOT cause a preflight
 PASS Content-Language header value with all allowed non-alphanumeric characters SHOULD NOT cause a preflight
 PASS Accept header with abnormal value SHOULD cause a preflight
 PASS Accept-Language header with abnormal value SHOULD cause a preflight
@@ -18,4 +36,17 @@
 PASS Content-Language header with abnormal value and explicitly allowed headers SHOULD be allowed
 PASS Accept header with normal value, Accept-Language header with normal value, Content-Language header with abnormal value, and explicitly allowed headers SHOULD be allowed
 PASS Accept header with normal value, then another Accept header with abnormal value, and explicitly allowed headers SHOULD be allowed
+PASS Accept header with disallowed delimiter '"' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '(' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter ')' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter ':' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '<' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '>' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '?' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '@' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '[' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '\' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter ']' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '{' SHOULD cause a preflight
+PASS Accept header with disallowed delimiter '}' SHOULD cause a preflight
 

Modified: trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html (210076 => 210077)


--- trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html	2016-12-21 22:02:48 UTC (rev 210076)
+++ trunk/LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html	2016-12-21 22:06:22 UTC (rev 210077)
@@ -34,7 +34,8 @@
     }
 
     var abnormalSimpleCorsHeaderValue = "() { :;};"
-    var allAllowedNonAlphanumericCharactersForAcceptHeader = " *,./;="
+    var allAllowedDelimiterCharactersForAcceptHeader = ",/;="
+    var allDisallowedDelimiterCharactersForAcceptHeader = ['"', '(', ')', ':', '<', '>', '?', '@', '[', '\\', ']', '{', '}'];
     var allAllowedNonAlphanumericCharactersForAcceptAndContentLanguageHeader = " *,-.;="
     var testCases = [
         // Positive test cases with normal headers
@@ -42,21 +43,45 @@
             headersToAdd: [{ name : "Accept", value: "application/json,text/*,*/*" }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
-            description: "Accept header with normal value SHOULD NOT cause a preflight"
+            description: "Accept header value 'application/json,text/*,*/*' SHOULD NOT cause a preflight"
         }
         ,{
-            headersToAdd: [{ name : "Accept", value: allAllowedNonAlphanumericCharactersForAcceptHeader }],
+            headersToAdd: [{ name : "Accept", value: "application/vnd.api+json" }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
-            description: "Accept header value with all allowed non-alphanumeric characters SHOULD NOT cause a preflight"
+            description: "Accept header with normal value 'application/vnd.api+json' SHOULD NOT cause a preflight"
         }
         ,{
+            headersToAdd: [{ name : "Accept", value: "text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c" }],
+            explicitlyAllowHeaders: false,
+            shouldCausePreflight: false,
+            description: "Accept header with normal value 'text/plain; q=0.5, text/html, text/x-dvi; q=0.8, text/x-c' SHOULD NOT cause a preflight"
+        }
+        ,{
+            headersToAdd: [{ name : "Accept", value: "text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5" }],
+            explicitlyAllowHeaders: false,
+            shouldCausePreflight: false,
+            description: "Accept header with normal value 'text/*;q=0.3, text/html;q=0.7, text/html;level=1, text/html;level=2;q=0.4, */*;q=0.5' SHOULD NOT cause a preflight"
+        }
+        ,{
+            headersToAdd: [{ name : "Accept", value: allAllowedDelimiterCharactersForAcceptHeader }],
+            explicitlyAllowHeaders: false,
+            shouldCausePreflight: false,
+            description: "Accept header value with all allowed delimiter characters SHOULD NOT cause a preflight"
+        }
+        ,{
             headersToAdd: [{ name : "Accept-Language", value: "en-US,en;q=0.8" }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
-            description: "Accept-Language header with normal value SHOULD NOT cause a preflight"
+            description: "Accept-Language header value 'en-US,en;q=0.8' SHOULD NOT cause a preflight"
         }
         ,{
+            headersToAdd: [{ name : "Accept-Language", value: "zh-Latn-CN-variant1-a-extend1-x-wadegile-private1" }],
+            explicitlyAllowHeaders: false,
+            shouldCausePreflight: false,
+            description: "Accept-Language header value 'zh-Latn-CN-variant1-a-extend1-x-wadegile-private1' SHOULD NOT cause a preflight"
+        }
+        ,{
             headersToAdd: [{ name : "Accept-Language", value: allAllowedNonAlphanumericCharactersForAcceptAndContentLanguageHeader }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
@@ -63,12 +88,18 @@
             description: "Accept-Language header value with all allowed non-alphanumeric characters SHOULD NOT cause a preflight"
         }
         ,{
-            headersToAdd: [{ name : "Content-Language", value: "en" }],
+            headersToAdd: [{ name : "Content-Language", value: "en-US,en;q=0.8" }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
-            description: "Content-Language header with normal value SHOULD NOT cause a preflight"
+            description: "Content-Language header value 'en-US,en;q=0.8' SHOULD NOT cause a preflight"
         }
         ,{
+            headersToAdd: [{ name : "Content-Language", value: "zh-Latn-CN-variant1-a-extend1-x-wadegile-private1" }],
+            explicitlyAllowHeaders: false,
+            shouldCausePreflight: false,
+            description: "Content-Language header value 'zh-Latn-CN-variant1-a-extend1-x-wadegile-private1' SHOULD NOT cause a preflight"
+        }
+        ,{
             headersToAdd: [{ name : "Content-Language", value: allAllowedNonAlphanumericCharactersForAcceptAndContentLanguageHeader }],
             explicitlyAllowHeaders: false,
             shouldCausePreflight: false,
@@ -132,6 +163,19 @@
         }
     ];
 
+    // Individual negative test cases for each disallowed delimiter character in Accept header values.
+    for (var i = 0; i < allDisallowedDelimiterCharactersForAcceptHeader.length; i++) {
+        var disallowedDelimiter = allDisallowedDelimiterCharactersForAcceptHeader[i];
+        testCases.push(
+            {
+                headersToAdd: [{ name : "Accept", value: disallowedDelimiter }],
+                explicitlyAllowHeaders: false,
+                shouldCausePreflight: true,
+                description: "Accept header with disallowed delimiter '" + disallowedDelimiter + "' SHOULD cause a preflight"
+            }
+        );
+    }
+
     function runTestCase(testNumber) {
         var testCase = testCases[testNumber];
         xhr = new XMLHttpRequest();
@@ -157,4 +201,4 @@
     runTestCase(0);
 </script>
 </body>
-</html>
\ No newline at end of file
+</html>

Modified: trunk/Source/WebCore/ChangeLog (210076 => 210077)


--- trunk/Source/WebCore/ChangeLog	2016-12-21 22:02:48 UTC (rev 210076)
+++ trunk/Source/WebCore/ChangeLog	2016-12-21 22:06:22 UTC (rev 210077)
@@ -1,3 +1,20 @@
+2016-12-21  John Wilander  <[email protected]>
+
+        Switch to a blacklist model for restricted Accept headers in simple CORS requests
+        https://bugs.webkit.org/show_bug.cgi?id=166363
+
+        Reviewed by Alex Christensen.
+
+        Updated existing tests.
+
+        * platform/network/HTTPParsers.cpp:
+        (WebCore::isDelimiterCharacter):
+            Convenience function for checking delimiter characters according to:
+            https://tools.ietf.org/html/rfc7230#section-3.2.6 
+        (WebCore::isValidAcceptHeaderValue):
+            Now uses WebCore::isDelimiterCharacter() to blacklist delimiter characters
+            instead of a whitelist of accepted non-alphanumeric characters.
+
 2016-12-21  Beth Dakin  <[email protected]>
 
         Holding down on candidates in the TouchBar should show panel on screen

Modified: trunk/Source/WebCore/platform/network/HTTPParsers.cpp (210076 => 210077)


--- trunk/Source/WebCore/platform/network/HTTPParsers.cpp	2016-12-21 22:02:48 UTC (rev 210076)
+++ trunk/Source/WebCore/platform/network/HTTPParsers.cpp	2016-12-21 22:06:22 UTC (rev 210077)
@@ -127,20 +127,31 @@
     return true;
 }
 
-// See RFC 7231, Section 5.3.2
+// See RFC 7230, Section 3.2.6.
+static bool isDelimiterCharacter(const UChar c)
+{
+    // DQUOTE and "(),/:;<=>?@[\]{}"
+    return (c == '"' || c == '(' || c == ')' || c == ',' || c == '/' || c == ':' || c == ';'
+        || c == '<' || c == '=' || c == '>' || c == '?' || c == '@' || c == '[' || c == '\\'
+        || c == ']' || c == '{' || c == '}');
+}
+
+// See RFC 7231, Section 5.3.2.
 bool isValidAcceptHeaderValue(const String& value)
 {
     for (unsigned i = 0; i < value.length(); ++i) {
         UChar c = value[i];
-        if (isASCIIAlphanumeric(c) || c == ' ' || c == '*' || c == ',' || c == '.' || c == '/' || c == ';' || c == '=')
+        // First check for alphanumeric for performance reasons then whitelist four delimiter characters.
+        if (isASCIIAlphanumeric(c) || c == ',' || c == '/' || c == ';' || c == '=')
             continue;
-        return false;
+        if (isDelimiterCharacter(c))
+            return false;
     }
     
     return true;
 }
 
-// See RFC 7231, Section 5.3.5 and 3.1.3.2
+// See RFC 7231, Section 5.3.5 and 3.1.3.2.
 bool isValidLanguageHeaderValue(const String& value)
 {
     for (unsigned i = 0; i < value.length(); ++i) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to