Fix #78929: plus signs in cookie values are converted to spaces

We switch the cookie value parsing function from `php_url_decode()` to
`php_raw_url_decode()`, so that cookie values are now parsed according
to RFC 6265, section 4.1.1.  We also refactor to remove duplicate code
without changing the execution flow.

diff --git a/NEWS b/NEWS
index 912b0c0b865bdef5eb0410b21109eb0afe9244fa..7372fb6a8721aa903f05120e49c58dc2ae7d9c58 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,10 @@ PHP                                                                        NEWS
 
 ?? ??? ????, PHP 7.4.2
 
+- Core:
+  . Fixed bug #78929 (plus signs in cookie values are converted to spaces).
+    (Alexey Kachalin)
+
 - OPcache:
   . Fixed bug #78950 (Preloading trait method with static variables). (Nikita)
 

diff --git a/main/php_variables.c b/main/php_variables.c
index 4b30d84f2feac6107a5f958d1e2a82204ed84046..5f6f1e5a096b216c9ae26bf54527e830391e16b4 100644 (file)
--- a/main/php_variables.c
+++ b/main/php_variables.c
@@ -479,6 +479,9 @@ SAPI_API SAPI_TREAT_DATA_FUNC(php_default_treat_data)
        var = php_strtok_r(res, separator, &strtok_buf);
 
        while (var) {
+               size_t val_len;
+               size_t new_val_len;
+
                val = strchr(var, '=');
 
                if (arg == PARSE_COOKIE) {
@@ -497,29 +500,25 @@ SAPI_API SAPI_TREAT_DATA_FUNC(php_default_treat_data)
                }
 
                if (val) { /* have a value */
-                       size_t val_len;
-                       size_t new_val_len;
 
                        *val++ = '\0';
-                       php_url_decode(var, strlen(var));
-                       val_len = php_url_decode(val, strlen(val));
-                       val = estrndup(val, val_len);
-                       if (sapi_module.input_filter(arg, var, &val, val_len, &new_val_len)) {
-                               php_register_variable_safe(var, val, new_val_len, &array);
+
+                       if (arg == PARSE_COOKIE) {
+                               val_len = php_raw_url_decode(val, strlen(val));
+                       } else {
+                               val_len = php_url_decode(val, strlen(val));
                        }
-                       efree(val);
                } else {
-                       size_t val_len;
-                       size_t new_val_len;
-
-                       php_url_decode(var, strlen(var));
-                       val_len = 0;
-                       val = estrndup("", val_len);
-                       if (sapi_module.input_filter(arg, var, &val, val_len, &new_val_len)) {
-                               php_register_variable_safe(var, val, new_val_len, &array);
-                       }
-                       efree(val);
+                       val     = "";
+                       val_len =  0;
+               }
+
+               val = estrndup(val, val_len);
+               php_url_decode(var, strlen(var));
+               if (sapi_module.input_filter(arg, var, &val, val_len, &new_val_len)) {
+                       php_register_variable_safe(var, val, new_val_len, &array);
                }
+               efree(val);
 next_cookie:
                var = php_strtok_r(NULL, separator, &strtok_buf);
        }

diff --git a/tests/basic/bug78929.phpt b/tests/basic/bug78929.phpt
new file mode 100644 (file)
index 0000000..60b71d1
--- /dev/null
+++ b/tests/basic/bug78929.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Bug #78929 (plus signs in cookie values are converted to spaces)
+--INI--
+max_input_vars=1000
+filter.default=unsafe_raw
+--COOKIE--
+RFC6265=#$%&'()*+-./0123456789<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~!
+--FILE--
+<?php
+var_dump($_COOKIE);
+?>
+--EXPECT--
+array(1) {
+  ["RFC6265"]=>
+  string(89) "#$%&'()*+-./0123456789<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmnopqrstuvwxyz{|}~!"
+}