diff --git a/ext/session/mod_user.c b/ext/session/mod_user.c index 71b8abdea8b0..4091589b54df 100644 --- a/ext/session/mod_user.c +++ b/ext/session/mod_user.c @@ -124,15 +124,25 @@ PS_CLOSE_FUNC(user) PS(mod_user_implemented) = false; + if (!bailout) { + ret = verify_bool_return_type_userland_calls(&retval); + } + if (!Z_ISUNDEF(retval)) { + zval_ptr_dtor(&retval); + } + + /* User close() may return false without calling parent::close(). */ + if (PS(default_mod) && PS(mod_data)) { + zend_try { + PS(default_mod)->s_close(&PS(mod_data)); + } zend_end_try(); + } + PS(mod_user_is_open) = false; + if (bailout) { - if (!Z_ISUNDEF(retval)) { - zval_ptr_dtor(&retval); - } zend_bailout(); } - ret = verify_bool_return_type_userland_calls(&retval); - zval_ptr_dtor(&retval); return ret; } diff --git a/ext/session/session.c b/ext/session/session.c index 1fdfc5d1073f..ae5e82be18bb 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -1767,8 +1767,19 @@ static bool php_session_abort(void) { if (PS(session_status) == php_session_active) { if (PS(mod_data) || PS(mod_user_implemented)) { - PS(mod)->s_close(&PS(mod_data)); + zend_try { + PS(mod)->s_close(&PS(mod_data)); + } zend_end_try(); + } + if (PS(id)) { + zend_string_release_ex(PS(id), false); + PS(id) = NULL; + } + if (PS(session_vars)) { + zend_string_release_ex(PS(session_vars), false); + PS(session_vars) = NULL; } + php_session_cleanup_filename(); PS(session_status) = php_session_none; return true; } diff --git a/ext/session/tests/session_abort_validateid_leak.phpt b/ext/session/tests/session_abort_validateid_leak.phpt new file mode 100644 index 000000000000..2e96aa524bdc --- /dev/null +++ b/ext/session/tests/session_abort_validateid_leak.phpt @@ -0,0 +1,34 @@ +--TEST-- +Session abort does not leak when validateId() returns wrong type +--INI-- +session.use_strict_mode=1 +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- +getMessage(), "\n"; +} + +session_write_close(); + +try { + session_start(); +} catch (Throwable $e) { + echo $e->getMessage(), "\n"; +} +?> +--EXPECTF-- +Session id must be a string \ No newline at end of file diff --git a/ext/session/tests/session_close_false_leak.phpt b/ext/session/tests/session_close_false_leak.phpt new file mode 100644 index 000000000000..e0674cbbe9fa --- /dev/null +++ b/ext/session/tests/session_close_false_leak.phpt @@ -0,0 +1,20 @@ +--TEST-- +Session close handler returning false does not leak memory +--INI-- +session.gc_probability=0 +--EXTENSIONS-- +session +--FILE-- + +--EXPECT-- \ No newline at end of file