Skip to content

Commit 4264db1

Browse files
committed
ext/session: report a rejected session cookie header
php_session_send_cookie() discarded the result of sapi_add_header_ex() and always returned SUCCESS. When the SAPI refuses a Set-Cookie header carrying CR or LF, the warning it emits can reach a userland error handler that calls session_destroy(), and php_session_reset_id() then appends the released PS(id). Return what sapi_add_header_ex() reports so the caller stops before touching session state again. Closes GH-22923
1 parent 2804d09 commit 4264db1

2 files changed

Lines changed: 33 additions & 2 deletions

File tree

ext/session/session.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,10 +1497,10 @@ static zend_result php_session_send_cookie(void) /* {{{ */
14971497
php_session_remove_cookie(); /* remove already sent session ID cookie */
14981498
/* 'replace' must be 0 here, else a previous Set-Cookie
14991499
header, probably sent with setcookie() will be replaced! */
1500-
sapi_add_header_ex(estrndup(ZSTR_VAL(ncookie.s), ZSTR_LEN(ncookie.s)), ZSTR_LEN(ncookie.s), 0, 0);
1500+
zend_result result = sapi_add_header_ex(estrndup(ZSTR_VAL(ncookie.s), ZSTR_LEN(ncookie.s)), ZSTR_LEN(ncookie.s), 0, 0);
15011501
smart_str_free(&ncookie);
15021502

1503-
return SUCCESS;
1503+
return result;
15041504
}
15051505
/* }}} */
15061506

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
session_start() when the SAPI rejects the session cookie header
3+
--INI--
4+
session.save_handler=files
5+
session.name=PHPSESSID
6+
session.gc_probability=0
7+
--EXTENSIONS--
8+
session
9+
--FILE--
10+
<?php
11+
12+
ob_start();
13+
14+
set_error_handler(function (int $errno, string $errstr): bool {
15+
echo "handler: ", $errstr, PHP_EOL;
16+
if (session_status() === PHP_SESSION_ACTIVE) {
17+
session_destroy();
18+
}
19+
return true;
20+
});
21+
22+
session_set_cookie_params(['path' => "/\r\nX-Injected: yes"]);
23+
24+
var_dump(session_start());
25+
var_dump(session_status() === PHP_SESSION_NONE);
26+
27+
?>
28+
--EXPECT--
29+
handler: Header may not contain more than a single header, new line detected
30+
bool(false)
31+
bool(true)

0 commit comments

Comments
 (0)