Switch to a blacklist model for restricted Accept headers in simple CORS requests
authorwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Dec 2016 22:06:22 +0000 (22:06 +0000)
committerwilander@apple.com <wilander@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 21 Dec 2016 22:06:22 +0000 (22:06 +0000)
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:

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

LayoutTests/ChangeLog
LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight-expected.txt
LayoutTests/http/tests/xmlhttprequest/cors-non-standard-safelisted-headers-should-trigger-preflight.html
Source/WebCore/ChangeLog
Source/WebCore/platform/network/HTTPParsers.cpp

index f7f9e8a..1f74381 100644 (file)
@@ -1,3 +1,13 @@
+2016-12-21  John Wilander  <wilander@apple.com>
+
+        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  <wenson_hsieh@apple.com>
 
         Add a layout test for scroll snapping with padding in the container
index 24107fe..3d9efaf 100644 (file)
@@ -3,11 +3,29 @@ CONSOLE MESSAGE: XMLHttpRequest cannot load http://localhost:8000/xmlhttprequest
 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 Accept header with abnormal value and explicitly allowed headers SHOULD be
 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
 
index 55f9caa..3e7c68a 100644 (file)
@@ -34,7 +34,8 @@
     }
 
     var abnormalSimpleCorsHeaderValue = "() { :;};"
-    var allAllowedNonAlphanumericCharactersForAcceptHeader = " *,./;="
+    var allAllowedDelimiterCharactersForAcceptHeader = ",/;="
+    var allDisallowedDelimiterCharactersForAcceptHeader = ['"', '(', ')', ':', '<', '>', '?', '@', '[', '\\', ']', '{', '}'];
     var allAllowedNonAlphanumericCharactersForAcceptAndContentLanguageHeader = " *,-.;="
     var testCases = [
         // Positive test cases with normal headers
             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 }],
             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 }],
         }
     ];
 
+    // 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();
     runTestCase(0);
 </script>
 </body>
-</html>
\ No newline at end of file
+</html>
index 611d3f7..ae48caf 100644 (file)
@@ -1,3 +1,20 @@
+2016-12-21  John Wilander  <wilander@apple.com>
+
+        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  <bdakin@apple.com>
 
         Holding down on candidates in the TouchBar should show panel on screen
index 8e1ea0b..158e2d9 100644 (file)
@@ -127,20 +127,31 @@ bool isValidHTTPHeaderValue(const String& value)
     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) {