Skip to content

broken session id code can cause zend_mm_heap corrupted #23043

Closed

Description

@lmaltsis

Description

The following code:

<?
ob_start();
class a extends SessionHandler {
    function open($d, $name): bool {
        return parent::open($d, $name);
    }
    function read($b): string {
        return "";
    }
    function create_sid(): string {
        return pathinfo(session_id())['filename'];
    }
}
$c = new a;
session_set_save_handler($c);
session_start();
session_write_close();
session_start();

Resulted in this output (~30% of the time):

zend_mm_heap corrupted
Segmentation fault (core dumped)

But I expected this output instead (this already happens ~70% of the time):

Warning: SessionHandler::write(): Session ID is too long or contains illegal characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in /tmp/bug.php on line 17

Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: a::write) in /tmp/bug.php on line 17

Warning: SessionHandler::write(): Session ID is too long or contains illegal characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed in Unknown on line 0

Warning: session_write_close(): Failed to write session data using user defined save handler. (session.save_path: , handler: a::write) in Unknown on line 0

PHP Version

PHP 8.5.9 (cli) (built: Aug  3 2026 17:04:37) (NTS DEBUG)
Copyright (c) The PHP Group
Zend Engine v4.5.9, Copyright (c) Zend Technologies
    with Zend OPcache v8.5.9, Copyright (c), by Zend Technologies

Also triggers on newest trunk https://github.com/php/php-src/commit/588dd04

Operating System

Ubuntu 24.04

Activity

  1. ndossche commented on Aug 4, 2026

    @ndossche
    Member

    Straightforward:

    diff --git a/ext/session/session.c b/ext/session/session.c
    index ba71d709a53..6380505ae95 100644
    --- a/ext/session/session.c
    +++ b/ext/session/session.c
    @@ -443,6 +443,7 @@ static zend_result php_session_initialize(void) /* {{{ */
     	if (!PS(id) || !ZSTR_VAL(PS(id))[0]) {
     		if (PS(id)) {
     			zend_string_release_ex(PS(id), 0);
    +			PS(id) = NULL;
     		}
     		PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
     		if (!PS(id)) {
    @@ -460,6 +461,7 @@ static zend_result php_session_initialize(void) /* {{{ */
     	) {
     		if (PS(id)) {
     			zend_string_release_ex(PS(id), 0);
    +			PS(id) = NULL;
     		}
     		PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
     		if (!PS(id)) {
    @@ -2440,6 +2442,7 @@ PHP_FUNCTION(session_regenerate_id)
     			/* Try to generate non-existing ID */
     			while (limit-- && PS(mod)->s_validate_sid(&PS(mod_data), PS(id)) == SUCCESS) {
     				zend_string_release_ex(PS(id), 0);
    +				PS(id) = NULL;
     				PS(id) = PS(mod)->s_create_sid(&PS(mod_data));
     				if (!PS(id)) {
     					PS(mod)->s_close(&PS(mod_data));
    

    Or if the old (invalid) session id must remain visible until after the call, then PS(id) must be stored in a local and released after the call. However, since the id is either missing or invalid at the point of the call, I presume that the above patch suffices.

  2. added a commit that references this issue on Aug 5, 2026
    be6fa2b
  3. added 2 commits that reference this issue on Aug 5, 2026
    9366c61
    0db3910
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions