Skip to content

Commit d7c98ca

Browse files
committed
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.
1 parent 6dd85f8 commit d7c98ca

File tree

4 files changed

+33
-9
lines changed

4 files changed

+33
-9
lines changed

NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ PHP NEWS
1414
. Fixed bug #80713 (SegFault when disabling ATTR_EMULATE_PREPARES and
1515
MySQL 8.0). (Nikita)
1616

17+
- Session:
18+
. Fixed bug #80774 (session_name() problem with backslash). (cmb)
19+
1720
04 Mar 2021, php 7.4.16
1821

1922
- Core:

ext/session/session.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,13 +1261,11 @@ static void php_session_remove_cookie(void) {
12611261
zend_llist_element *next;
12621262
zend_llist_element *current;
12631263
char *session_cookie;
1264-
zend_string *e_session_name;
12651264
size_t session_cookie_len;
12661265
size_t len = sizeof("Set-Cookie")-1;
12671266

1268-
e_session_name = php_url_encode(PS(session_name), strlen(PS(session_name)));
1269-
spprintf(&session_cookie, 0, "Set-Cookie: %s=", ZSTR_VAL(e_session_name));
1270-
zend_string_free(e_session_name);
1267+
ZEND_ASSERT(strpbrk(PS(session_name), "=,; \t\r\n\013\014") == NULL);
1268+
spprintf(&session_cookie, 0, "Set-Cookie: %s=", PS(session_name));
12711269

12721270
session_cookie_len = strlen(session_cookie);
12731271
current = l->head;
@@ -1299,7 +1297,7 @@ static int php_session_send_cookie(void) /* {{{ */
12991297
{
13001298
smart_str ncookie = {0};
13011299
zend_string *date_fmt = NULL;
1302-
zend_string *e_session_name, *e_id;
1300+
zend_string *e_id;
13031301

13041302
if (SG(headers_sent)) {
13051303
const char *output_start_filename = php_output_get_start_filename();
@@ -1313,16 +1311,20 @@ static int php_session_send_cookie(void) /* {{{ */
13131311
return FAILURE;
13141312
}
13151313

1316-
/* URL encode session_name and id because they might be user supplied */
1317-
e_session_name = php_url_encode(PS(session_name), strlen(PS(session_name)));
1314+
/* Prevent broken Set-Cookie header, because the session_name might be user supplied */
1315+
if (strpbrk(PS(session_name), "=,; \t\r\n\013\014") != NULL) { /* man isspace for \013 and \014 */
1316+
php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,; \\t\\r\\n\\013\\014'");
1317+
return FAILURE;
1318+
}
1319+
1320+
/* URL encode id because it might be user supplied */
13181321
e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id)));
13191322

13201323
smart_str_appendl(&ncookie, "Set-Cookie: ", sizeof("Set-Cookie: ")-1);
1321-
smart_str_appendl(&ncookie, ZSTR_VAL(e_session_name), ZSTR_LEN(e_session_name));
1324+
smart_str_appendl(&ncookie, PS(session_name), strlen(PS(session_name)));
13221325
smart_str_appendc(&ncookie, '=');
13231326
smart_str_appendl(&ncookie, ZSTR_VAL(e_id), ZSTR_LEN(e_id));
13241327

1325-
zend_string_release_ex(e_session_name, 0);
13261328
zend_string_release_ex(e_id, 0);
13271329

13281330
if (PS(cookie_lifetime) > 0) {

ext/session/tests/bug80774.phpt

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
Bug #80774 (session_name() problem with backslash)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('session')) die("skip session extension not available");
6+
?>
7+
--FILE--
8+
<?php
9+
session_name("foo\\bar");
10+
session_id('12345');
11+
session_start();
12+
?>
13+
--EXPECTHEADERS--
14+
Set-Cookie: foo\bar=12345; path=/
15+
--EXPECT--

ext/session/tests/session_name_variation1.phpt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,17 @@ string(9) "PHPSESSID"
4848
bool(true)
4949
string(9) "PHPSESSID"
5050
string(9) "PHPSESSID"
51+
52+
Warning: session_start(): session.name cannot contain any of the following '=,; \t\r\n\013\014' in %s on line %d
5153
bool(true)
5254
string(1) " "
5355
bool(true)
5456
string(1) " "
5557

5658
Warning: session_name(): session.name cannot be a numeric or empty '' in %s on line %d
5759
string(1) " "
60+
61+
Warning: session_start(): session.name cannot contain any of the following '=,; \t\r\n\013\014' in %s on line %d
5862
bool(true)
5963
string(1) " "
6064
bool(true)

0 commit comments

Comments
 (0)