From: Kachalin Alexey <kachalin.alexey@gmail.com>
Date: Thu, 12 Dec 2019 10:49:06 +0000 (+0100)
Subject: Fix #78929: plus signs in cookie values are converted to spaces
X-Git-Tag: php-7.4.7RC1~431
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=79376ab209f61be03bbf8c1b6177c18261767da8;p=php

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 912b0c0b86..7372fb6a87 100644
--- 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 4b30d84f2f..5f6f1e5a09 100644
--- 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
index 0000000000..60b71d1f8f
--- /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{|}~!"
+}