Fix #80774: session_name() problem with backslash

Since we do no longer URL decode cookie names[1], we must not URL
encode the session name.  We need to prevent broken Set-Cookie headers,
by rejecting names which contain invalid characters.

[1] <http://git.php.net/?p=php-src.git;a=commit;h=6559fe912661ca5ce5f0eeeb591d928451428ed0>

Closes GH-6711.

diff --git a/NEWS b/NEWS
index 3cb29d3cb49e7f53cca808f5739e07f6047531ed..9c69fcd16b4ff6e655b42fc802be4d87c10ee78b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -14,6 +14,9 @@ PHP                                                                        NEWS
   . Fixed bug #80713 (SegFault when disabling ATTR_EMULATE_PREPARES and
     MySQL 8.0). (Nikita)
 
+- Session:
+  . Fixed bug #80774 (session_name() problem with backslash). (cmb)
+
 04 Mar 2021, php 7.4.16
 
 - Core:

diff --git a/ext/session/session.c b/ext/session/session.c
index 1efe220c77d042b8420c848b60f3a90846909f2a..d652383b4560b0bf267684054f21cd63cd01dabf 100644 (file)
--- a/ext/session/session.c
+++ b/ext/session/session.c
@@ -1261,13 +1261,11 @@ static void php_session_remove_cookie(void) {
        zend_llist_element *next;
        zend_llist_element *current;
        char *session_cookie;
-       zend_string *e_session_name;
        size_t session_cookie_len;
        size_t len = sizeof("Set-Cookie")-1;
 
-       e_session_name = php_url_encode(PS(session_name), strlen(PS(session_name)));
-       spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(e_session_name));
-       zend_string_free(e_session_name);
+       ZEND_ASSERT(strpbrk(PS(session_name), "=,; \t\r\n\013\014") == NULL);
+       spprintf(&session_cookie, 0, "Set-Cookie: %s=", PS(session_name));
 
        session_cookie_len = strlen(session_cookie);
        current = l->head;
@@ -1299,7 +1297,7 @@ static int php_session_send_cookie(void) /* {{{ */
 {
        smart_str ncookie = {0};
        zend_string *date_fmt = NULL;
-       zend_string *e_session_name, *e_id;
+       zend_string *e_id;
 
        if (SG(headers_sent)) {
                const char *output_start_filename = php_output_get_start_filename();
@@ -1313,16 +1311,20 @@ static int php_session_send_cookie(void) /* {{{ */
                return FAILURE;
        }
 
-       /* URL encode session_name and id because they might be user supplied */
-       e_session_name = php_url_encode(PS(session_name), strlen(PS(session_name)));
+       /* Prevent broken Set-Cookie header, because the session_name might be user supplied */
+       if (strpbrk(PS(session_name), "=,; \t\r\n\013\014") != NULL) {   /* man isspace for \013 and \014 */
+               php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,; \\t\\r\\n\\013\\014'");
+               return FAILURE;
+       }
+
+       /* URL encode id because it might be user supplied */
        e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)));
 
        smart_str_appendl(&ncookie, "Set-Cookie: ", sizeof("Set-Cookie: ")-1);
-       smart_str_appendl(&ncookie, ZSTR_VAL(e_session_name), ZSTR_LEN(e_session_name));
+       smart_str_appendl(&ncookie, PS(session_name), strlen(PS(session_name)));
        smart_str_appendc(&ncookie, '=');
        smart_str_appendl(&ncookie, ZSTR_VAL(e_id), ZSTR_LEN(e_id));
 
-       zend_string_release_ex(e_session_name, 0);
        zend_string_release_ex(e_id, 0);
 
        if (PS(cookie_lifetime) > 0) {

diff --git a/ext/session/tests/bug80774.phpt b/ext/session/tests/bug80774.phpt
new file mode 100644 (file)
index 0000000..2ce0726
--- /dev/null
+++ b/ext/session/tests/bug80774.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Bug #80774 (session_name() problem with backslash)
+--SKIPIF--
+<?php
+if (!extension_loaded('session')) die("skip session extension not available");
+?>
+--FILE--
+<?php
+session_name("foo\\bar");
+session_id('12345');
+session_start();
+?>
+--EXPECTHEADERS--
+Set-Cookie: foo\bar=12345; path=/
+--EXPECT--

diff --git a/ext/session/tests/session_name_variation1.phpt b/ext/session/tests/session_name_variation1.phpt
index 82e5278783e4f1dad369a995ee0f19a1fd04076e..cda7db31a5e26e416a45c63a70462009875ca919 100644 (file)
--- a/ext/session/tests/session_name_variation1.phpt
+++ b/ext/session/tests/session_name_variation1.phpt
@@ -48,6 +48,8 @@ string(9) "PHPSESSID"
 bool(true)
 string(9) "PHPSESSID"
 string(9) "PHPSESSID"
+
+Warning: session_start(): session.name cannot contain any of the following '=,; \t\r\n\013\014' in %s on line %d
 bool(true)
 string(1) "    "
 bool(true)
@@ -55,6 +57,8 @@ string(1) "   "
 
 Warning: session_name(): session.name cannot be a numeric or empty '' in %s on line %d
 string(1) "    "
+
+Warning: session_start(): session.name cannot contain any of the following '=,; \t\r\n\013\014' in %s on line %d
 bool(true)
 string(1) "    "
 bool(true)