Updated parsing of cookies, reject cookie completely if discipline is very strict

src/microhttpd/connection.c

@@ -2850,7 +2850,7 @@ parse_cookies_string (char *str,
   /* Allow whitespaces around '=' character */
   const bool wsp_around_eq = (-3 >= connection->daemon->client_discipline);
   /* Allow whitespaces in quoted cookie value */
-  const bool wsp_in_quoted = (0 >= connection->daemon->client_discipline);
+  const bool wsp_in_quoted = (-2 >= connection->daemon->client_discipline);
   /* Allow tab as space after semicolon between cookies */
   const bool tab_as_sp = (0 >= connection->daemon->client_discipline);
   /* Allow no space after semicolon between cookies */
@@ -3048,8 +3048,10 @@ parse_cookie_header (struct MHD_Connection *connection)
   char *cpy;
   size_t i;
   enum _MHD_ParseCookie parse_res;
-  const struct MHD_HTTP_Req_Header *const saved_tail =
+  struct MHD_HTTP_Req_Header *const saved_tail =
     connection->rq.headers_received_tail;
+  const bool allow_partially_correct_cookie =
+    (1 >= connection->daemon->client_discipline);
 
   if (MHD_NO ==
       MHD_lookup_connection_value_n (connection,
@@ -3097,9 +3099,22 @@ parse_cookie_header (struct MHD_Connection *connection)
   case MHD_PARSE_COOKIE_MALFORMED:
 #ifdef HAVE_MESSAGES
     if (saved_tail != connection->rq.headers_received_tail)
-      MHD_DLOG (connection->daemon,
-                _ ("The Cookie header has been only partially parsed as it "
-                   "contains malformed data.\n"));
+    {
+      if (allow_partially_correct_cookie)
+        MHD_DLOG (connection->daemon,
+                  _ ("The Cookie header has been only partially parsed as it "
+                     "contains malformed data.\n"));
+      else
+      {
+        /* Remove extracted values from partially broken cookie */
+        /* Memory remains allocated until the end of the request processing */
+        connection->rq.headers_received_tail = saved_tail;
+        saved_tail->next = NULL;
+        MHD_DLOG (connection->daemon,
+                  _ ("The Cookie header has been ignored as it contains "
+                     "malformed data.\n"));
+      }
+    }
     else
       MHD_DLOG (connection->daemon,
                 _ ("The Cookie header has malformed data.\n"));

src/testcurl/Makefile.am

@@ -144,9 +144,11 @@ check_PROGRAMS = \
 
 if ENABLE_COOKIE
 check_PROGRAMS += \
-  test_parse_cookies_strict_p1 \
-  test_parse_cookies_strict_zero \
-  test_parse_cookies_strict_n1
+  test_parse_cookies_discp_p2 \
+  test_parse_cookies_discp_p1 \
+  test_parse_cookies_discp_zero \
+  test_parse_cookies_discp_n2 \
+  test_parse_cookies_discp_n3
 endif
 
 if HEAVY_TESTS
@@ -468,14 +470,20 @@ test_post_SOURCES = \
 test_process_headers_SOURCES = \
   test_process_headers.c mhd_has_in_name.h
 
-test_parse_cookies_strict_zero_SOURCES = \
+test_parse_cookies_discp_zero_SOURCES = \
   test_parse_cookies.c mhd_has_in_name.h mhd_has_param.h
 
-test_parse_cookies_strict_p1_SOURCES = \
-  $(test_parse_cookies_strict_zero_SOURCES)
+test_parse_cookies_discp_p2_SOURCES = \
+  $(test_parse_cookies_discp_zero_SOURCES)
 
-test_parse_cookies_strict_n1_SOURCES = \
-  $(test_parse_cookies_strict_zero_SOURCES)
+test_parse_cookies_discp_p1_SOURCES = \
+  $(test_parse_cookies_discp_zero_SOURCES)
+
+test_parse_cookies_discp_n2_SOURCES = \
+  $(test_parse_cookies_discp_zero_SOURCES)
+
+test_parse_cookies_discp_n3_SOURCES = \
+  $(test_parse_cookies_discp_zero_SOURCES)
 
 test_process_arguments_SOURCES = \
   test_process_arguments.c mhd_has_in_name.h

src/testcurl/test_parse_cookies.c

@@ -237,10 +237,11 @@ struct strct_test_data
 {
   unsigned int line_num;
   const char *header_str;
-  unsigned int num_cookies_strict_p2; /* Reserved */
+  unsigned int num_cookies_strict_p2;
   unsigned int num_cookies_strict_p1;
   unsigned int num_cookies_strict_zero;
-  unsigned int num_cookies_strict_n1;
+  unsigned int num_cookies_strict_n2;
+  unsigned int num_cookies_strict_n3;
   struct strct_cookie cookies[5];
 };
 
@@ -252,6 +253,7 @@ static const struct strct_test_data test_data[] = {
     1,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -263,6 +265,7 @@ static const struct strct_test_data test_data[] = {
   {
     __LINE__,
     "name1=value1;",
+    0,
     1,
     1,
     1,
@@ -282,6 +285,7 @@ static const struct strct_test_data test_data[] = {
     1,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -297,6 +301,7 @@ static const struct strct_test_data test_data[] = {
     0,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -312,6 +317,7 @@ static const struct strct_test_data test_data[] = {
     0,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -327,6 +333,7 @@ static const struct strct_test_data test_data[] = {
     1,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -342,6 +349,7 @@ static const struct strct_test_data test_data[] = {
     0,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -357,6 +365,7 @@ static const struct strct_test_data test_data[] = {
     0,
     1,
     1,
+    1,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_NULL,
@@ -370,6 +379,7 @@ static const struct strct_test_data test_data[] = {
     "name2=\"value 2\"",
     0,
     0,
+    0,
     1,
     1,
     {
@@ -387,6 +397,7 @@ static const struct strct_test_data test_data[] = {
     1,
     2,
     2,
+    2,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_ ("name2", "value2"),
@@ -402,9 +413,10 @@ static const struct strct_test_data test_data[] = {
     2,
     2,
     2,
+    2,
     {
       COOKIE_ ("name1", "value1"),
-      COOKIE_ ("name1", "value1"),
+      COOKIE_ ("name1", "value1"), /* The second value is not checked actually */
       COOKIE_NULL,
       COOKIE_NULL,
       COOKIE_NULL
@@ -417,6 +429,7 @@ static const struct strct_test_data test_data[] = {
     2,
     2,
     2,
+    2,
     {
       COOKIE_ ("name1", "value1"),
       COOKIE_ ("name2", "value2"),
@@ -427,7 +440,8 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
+    "name1=value1; name2=value2    ",
+    2,
     2,
     2,
     2,
@@ -442,8 +456,9 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
-    2,
+    "name1=value1;  name2=value2",
+    0,
+    1,
     2,
     2,
     2,
@@ -457,8 +472,9 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
-    2,
+    "name1=value1;name2=value2",
+    0,
+    1,
     2,
     2,
     2,
@@ -472,8 +488,9 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
-    2,
+    "name1=value1;\tname2=value2",
+    0,
+    1,
     2,
     2,
     2,
@@ -487,8 +504,9 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
-    2,
+    "name1=value1 ; name2=value2",
+    0,
+    0,
     2,
     2,
     2,
@@ -502,7 +520,8 @@ static const struct strct_test_data test_data[] = {
   },
   {
     __LINE__,
-    "name1=value1; name2=value2",
+    "     name1=value1; name2=value2",
+    2,
     2,
     2,
     2,
@@ -522,6 +541,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char",
     0,
     3,
+    3,
     5,
     5,
     {
@@ -539,6 +559,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char",
     0,
     1,
+    3,
     5,
     5,
     {
@@ -556,6 +577,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char\t \t",
     0,
     1,
+    3,
     5,
     5,
     {
@@ -573,6 +595,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char;\t \t",
     0,
     1,
+    3,
     5,
     5,
     {
@@ -590,6 +613,7 @@ static const struct strct_test_data test_data[] = {
     "name4=\"var4 with spaces\"",
     0,
     4,
+    4,
     5,
     5,
     {
@@ -607,6 +631,7 @@ static const struct strct_test_data test_data[] = {
     "name4=\"var4 with spaces\";",
     0,
     4,
+    4,
     5,
     5,
     {
@@ -624,6 +649,7 @@ static const struct strct_test_data test_data[] = {
     "name4=\"var4 with spaces\"; name3=",
     0,
     3,
+    3,
     5,
     5,
     {
@@ -641,6 +667,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char; name3=;",
     0,
     2,
+    2,
     5,
     5,
     {
@@ -658,6 +685,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char",
     0,
     0,
+    3,
     5,
     5,
     {
@@ -675,6 +703,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char",
     0,
     3,
+    3,
     5,
     5,
     {
@@ -692,6 +721,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char;;;;;;;;",
     0,
     3,
+    3,
     5,
     5,
     {
@@ -709,6 +739,7 @@ static const struct strct_test_data test_data[] = {
     "name5=var_with_=_char; ; ; ; ; name3=",
     0,
     2,
+    2,
     5,
     5,
     {
@@ -726,14 +757,15 @@ static const struct strct_test_data test_data[] = {
     "name4=\"var4 with spaces\" ",
     0,
     0,
+    4,
     5,
     5,
     {
       COOKIE_ ("name1", "var1"),
       COOKIE_ ("name2", "var2"),
       COOKIE_ ("name3", ""),
-      COOKIE_ ("name4", "var4 with spaces"),
-      COOKIE_ ("name5", "var_with_=_char")
+      COOKIE_ ("name5", "var_with_=_char"),
+      COOKIE_ ("name4", "var4 with spaces")
     }
   },
   {
@@ -742,6 +774,7 @@ static const struct strct_test_data test_data[] = {
     "name1=var1; name2=var2; name3=",
     0,
     1,
+    1,
     5,
     5,
     {
@@ -752,12 +785,30 @@ static const struct strct_test_data test_data[] = {
       COOKIE_ ("name4", "var4 with spaces")
     }
   },
+  {
+    __LINE__,
+    "name5=var_with_=_char; name4=\"var4_without_spaces\"; " \
+    "name1=var1; name2=var2; name3=",
+    5,
+    5,
+    5,
+    5,
+    5,
+    {
+      COOKIE_ ("name5", "var_with_=_char"),
+      COOKIE_ ("name1", "var1"),
+      COOKIE_ ("name2", "var2"),
+      COOKIE_ ("name3", ""),
+      COOKIE_ ("name4", "var4_without_spaces")
+    }
+  },
   {
     __LINE__,
     "name1 = value1",
     0,
     0,
     0,
+    0,
     1,
     {
       COOKIE_ ("name1", "value1"),
@@ -773,6 +824,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     1,
     {
       COOKIE_ ("name1", "value1"),
@@ -788,6 +840,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     1,
     {
       COOKIE_ ("name1", "value1"),
@@ -803,6 +856,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     2,
     {
       COOKIE_ ("name1", "value1"),
@@ -818,6 +872,7 @@ static const struct strct_test_data test_data[] = {
     0,
     1,
     1,
+    1,
     2,
     {
       COOKIE_ ("name1", "value1"),
@@ -833,6 +888,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     2,
     {
       COOKIE_ ("name1", "value1"),
@@ -849,6 +905,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -864,6 +921,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -879,6 +937,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -894,6 +953,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -909,6 +969,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -924,6 +985,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -939,6 +1001,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -954,6 +1017,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -969,6 +1033,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -984,6 +1049,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -999,6 +1065,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -1014,6 +1081,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -1029,6 +1097,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -1044,6 +1113,7 @@ static const struct strct_test_data test_data[] = {
     0,
     0,
     0,
+    0,
     {
       COOKIE_NULL,
       COOKIE_NULL,
@@ -1057,10 +1127,12 @@ static const struct strct_test_data test_data[] = {
 /* Global parameters */
 static int verbose;
 static int oneone;                  /**< If false use HTTP/1.0 for requests*/
-static int use_strict_n1;
-static int use_strict_zero;
-static int use_strict_p1;
-static int strict_level;
+static int use_discp_n3;
+static int use_discp_n2;
+static int use_discp_zero;
+static int use_discp_p1;
+static int use_discp_p2;
+static int discp_level;
 
 static void
 test_global_init (void)
@@ -1128,12 +1200,16 @@ ahcCheck (void *cls,
   unsigned int i;
   int cookie_failed;
 
-  if (use_strict_p1)
+  if (use_discp_p2)
+    expected_num_cookies = param->check->num_cookies_strict_p2;
+  else if (use_discp_p1)
     expected_num_cookies = param->check->num_cookies_strict_p1;
-  else if (use_strict_zero)
+  else if (use_discp_zero)
     expected_num_cookies = param->check->num_cookies_strict_zero;
-  else if (use_strict_n1)
-    expected_num_cookies = param->check->num_cookies_strict_n1;
+  else if (use_discp_n2)
+    expected_num_cookies = param->check->num_cookies_strict_n2;
+  else if (use_discp_n3)
+    expected_num_cookies = param->check->num_cookies_strict_n3;
   else
     externalErrorExit ();
 
@@ -1565,13 +1641,13 @@ testExternalPolling (void)
   if (MHD_NO != MHD_is_feature_supported (MHD_FEATURE_AUTODETECT_BIND_PORT))
     port = 0;
   else
-    port = 1340 + oneone ? 0 : 6 + (uint16_t) (1 + strict_level);
+    port = 1340 + oneone ? 0 : 6 + (uint16_t) (1 + discp_level);
 
   d = MHD_start_daemon (MHD_USE_ERROR_LOG,
                         port, NULL, NULL,
                         &ahcCheck, &ahc_param,
-                        MHD_OPTION_STRICT_FOR_CLIENT,
-                        (int) (strict_level),
+                        MHD_OPTION_CLIENT_DISCIPLINE_LVL,
+                        (int) (discp_level),
                         MHD_OPTION_END);
   if (d == NULL)
     return 1;
@@ -1640,19 +1716,26 @@ main (int argc, char *const *argv)
                has_param (argc, argv, "-s") ||
                has_param (argc, argv, "--silent"));
   oneone = ! has_in_name (argv[0], "10");
-  use_strict_n1 = has_in_name (argv[0], "_strict_n1");
-  use_strict_zero = has_in_name (argv[0], "_strict_zero");
-  use_strict_p1 = has_in_name (argv[0], "_strict_p1");
-  if (1 != ((use_strict_n1 ? 1 : 0) + (use_strict_zero ? 1 : 0)
-            + (use_strict_p1 ? 1 : 0)))
+  use_discp_n3 = has_in_name (argv[0], "_discp_n3");
+  use_discp_n2 = has_in_name (argv[0], "_discp_n2");
+  use_discp_zero = has_in_name (argv[0], "_discp_zero");
+  use_discp_p1 = has_in_name (argv[0], "_discp_p1");
+  use_discp_p2 = has_in_name (argv[0], "_discp_p2");
+  if (1 != ((use_discp_n3 ? 1 : 0) + (use_discp_n2 ? 1 : 0)
+            + (use_discp_zero ? 1 : 0)
+            + (use_discp_p1 ? 1 : 0) + (use_discp_p2 ? 1 : 0)))
     return 99;
 
-  if (use_strict_n1)
-    strict_level = -1;
-  else if (use_strict_zero)
-    strict_level = 0;
-  else if (use_strict_p1)
-    strict_level = 1;
+  if (use_discp_n3)
+    discp_level = -3;
+  else if (use_discp_n2)
+    discp_level = -2;
+  else if (use_discp_zero)
+    discp_level = 0;
+  else if (use_discp_p1)
+    discp_level = 1;
+  else if (use_discp_p2)
+    discp_level = 2;
 
   test_global_init ();