From 127ea1e873cbc55501053fc2a55c2fc5b7a2b0e7 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 29 Jul 2026 12:00:00 -0400 Subject: [PATCH 1/2] ext/session: do not reuse session state a save handler tore down session_regenerate_id() runs the userland write or destroy handler and then the close handler, and keeps operating on PS(id) afterwards. Any of them may call session_destroy(): php_session_destroy() runs php_rshutdown_session_globals() even when the recursive call was rejected, releasing PS(id) and setting it to NULL, so the following zend_string_release_ex() dereferences NULL. Check that the session is still active once every handler has run. Closes GH-22926 --- ext/session/session.c | 6 ++ ..._regenerate_id_handler_closes_session.phpt | 67 +++++++++++++++++++ ...egenerate_id_handler_destroys_session.phpt | 37 ++++++++++ 3 files changed, 110 insertions(+) create mode 100644 ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt create mode 100644 ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt diff --git a/ext/session/session.c b/ext/session/session.c index ba71d709a536..9105b7c6b273 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -2408,8 +2408,14 @@ PHP_FUNCTION(session_regenerate_id) RETURN_FALSE; } } + PS(mod)->s_close(&PS(mod_data)); + if (PS(session_status) != php_session_active) { + php_error_docref(NULL, E_WARNING, "Session ID cannot be regenerated because the save handler closed the session"); + RETURN_FALSE; + } + /* New session data */ if (PS(session_vars)) { zend_string_release_ex(PS(session_vars), 0); diff --git a/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt b/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt new file mode 100644 index 000000000000..d4908502f0ca --- /dev/null +++ b/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt @@ -0,0 +1,67 @@ +--TEST-- +session_regenerate_id() when the close handler destroys the session +--INI-- +session.save_handler=files +session.name=PHPSESSID +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- +destroyed) { + $this->destroyed = true; + session_destroy(); + } + return true; + } + + public function read(string $id): string|false + { + return ''; + } + + public function write(string $id, string $data): bool + { + return true; + } + + public function destroy(string $id): bool + { + return true; + } + + public function gc(int $max_lifetime): int|false + { + return 0; + } +} + +session_set_save_handler(new MySessionHandler(), true); +session_start(); + +var_dump(session_regenerate_id(false)); +var_dump(session_status() === PHP_SESSION_NONE); + +?> +--EXPECTF-- +Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d + +Warning: session_destroy(): Session object destruction failed in %s on line %d + +Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d +bool(false) +bool(true) diff --git a/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt b/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt new file mode 100644 index 000000000000..ae0fcfd8b7b2 --- /dev/null +++ b/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt @@ -0,0 +1,37 @@ +--TEST-- +session_regenerate_id() when the save handler destroys the session +--INI-- +session.save_handler=files +session.name=PHPSESSID +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- + +--EXPECTF-- +Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d + +Warning: session_destroy(): Session object destruction failed in %s on line %d + +Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d +bool(false) +bool(true) From f967d5ed996c458219e2459949451538177d03f1 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Wed, 29 Jul 2026 12:43:27 -0400 Subject: [PATCH 2/2] ext/session: hold the save handler recursion guard for the whole call ps_call_handler() cleared PS(in_save_handler) on the branch that rejects a recursive call, so the guard was released by the frame that was refused rather than by the frame that set it. One rejected nested call disarmed it and the next nested call from the same handler re-entered userland. Holding it exposes a second defect: php_rshutdown_session_globals() can no longer run SessionHandler::close(), and PS(default_mod)'s data is orphaned once PS(mod_data) is cleared. Release it there, which also covers a handler that never delegates close() to its parent. --- ext/session/mod_user.c | 1 - ext/session/session.c | 6 +++ .../recursive_handler_argv_leak.phpt | 2 + .../session_handler_close_without_parent.phpt | 31 +++++++++++ ..._regenerate_id_handler_closes_session.phpt | 2 + ...egenerate_id_handler_destroys_session.phpt | 2 + .../session_save_handler_recursion_guard.phpt | 51 +++++++++++++++++++ 7 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 ext/session/tests/user_session_module/session_handler_close_without_parent.phpt create mode 100644 ext/session/tests/user_session_module/session_save_handler_recursion_guard.phpt diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 71b18612683d..52c6d954587c 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -27,7 +27,6 @@ static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval) { int i; if (PS(in_save_handler)) { - PS(in_save_handler) = 0; ZVAL_UNDEF(retval); php_error_docref(NULL, E_WARNING, "Cannot call session save handler in a recursive manner"); } else { diff --git a/ext/session/session.c b/ext/session/session.c index 9105b7c6b273..8ba3c36a92e6 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -145,6 +145,12 @@ static void php_rshutdown_session_globals(void) /* {{{ */ PS(mod)->s_close(&PS(mod_data)); } zend_end_try(); } + if (PS(mod_user_is_open) && PS(default_mod) && PS(mod_data)) { + PS(mod_user_is_open) = 0; + zend_try { + PS(default_mod)->s_close(&PS(mod_data)); + } zend_end_try(); + } if (PS(id)) { zend_string_release_ex(PS(id), 0); PS(id) = NULL; diff --git a/ext/session/tests/user_session_module/recursive_handler_argv_leak.phpt b/ext/session/tests/user_session_module/recursive_handler_argv_leak.phpt index 2b954494e87c..1e869bdf4c2e 100644 --- a/ext/session/tests/user_session_module/recursive_handler_argv_leak.phpt +++ b/ext/session/tests/user_session_module/recursive_handler_argv_leak.phpt @@ -28,4 +28,6 @@ echo "done\n"; Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d Warning: session_destroy(): Session object destruction failed in %s on line %d + +Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d done diff --git a/ext/session/tests/user_session_module/session_handler_close_without_parent.phpt b/ext/session/tests/user_session_module/session_handler_close_without_parent.phpt new file mode 100644 index 000000000000..b5da5e48751d --- /dev/null +++ b/ext/session/tests/user_session_module/session_handler_close_without_parent.phpt @@ -0,0 +1,31 @@ +--TEST-- +SessionHandler::close() that is never delegated still releases the default handler +--INI-- +session.save_handler=files +session.name=PHPSESSID +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- + +--EXPECT-- +bool(true) +bool(true) diff --git a/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt b/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt index d4908502f0ca..40bebfc19e9f 100644 --- a/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt +++ b/ext/session/tests/user_session_module/session_regenerate_id_handler_closes_session.phpt @@ -62,6 +62,8 @@ Warning: session_destroy(): Cannot call session save handler in a recursive mann Warning: session_destroy(): Session object destruction failed in %s on line %d +Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d + Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d bool(false) bool(true) diff --git a/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt b/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt index ae0fcfd8b7b2..fe7f010855e3 100644 --- a/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt +++ b/ext/session/tests/user_session_module/session_regenerate_id_handler_destroys_session.phpt @@ -32,6 +32,8 @@ Warning: session_destroy(): Cannot call session save handler in a recursive mann Warning: session_destroy(): Session object destruction failed in %s on line %d +Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d + Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d bool(false) bool(true) diff --git a/ext/session/tests/user_session_module/session_save_handler_recursion_guard.phpt b/ext/session/tests/user_session_module/session_save_handler_recursion_guard.phpt new file mode 100644 index 000000000000..49d1cd6bbb02 --- /dev/null +++ b/ext/session/tests/user_session_module/session_save_handler_recursion_guard.phpt @@ -0,0 +1,51 @@ +--TEST-- +The save handler recursion guard survives a rejected nested call +--INI-- +session.save_handler=files +session.name=PHPSESSID +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- +gcEntered++; + return 0; + } + + public function write(string $id, string $data): bool + { + if ($this->armed) { + $this->armed = false; + var_dump(session_gc()); + var_dump(session_gc()); + } + return parent::write($id, $data); + } +} + +$handler = new MySessionHandler(); +session_set_save_handler($handler, true); +session_start(); +$_SESSION['key'] = 'value'; +session_write_close(); + +echo 'gc() entered ', $handler->gcEntered, " time(s)\n"; + +?> +--EXPECTF-- +Warning: session_gc(): Cannot call session save handler in a recursive manner in %s on line %d +bool(false) + +Warning: session_gc(): Cannot call session save handler in a recursive manner in %s on line %d +bool(false) +gc() entered 0 time(s)