Skip to content

ext/session: check the created ID before validating it - #197

Closed
iliaal wants to merge 1 commit into
PHP-8.4from
fix/session-create-id-null-sid
Closed

ext/session: check the created ID before validating it#197
iliaal wants to merge 1 commit into
PHP-8.4from
fix/session-create-id-null-sid

Conversation

@iliaal

@iliaal iliaal commented Jul 29, 2026

Copy link
Copy Markdown
Owner

session_create_id() hands the return value of s_create_sid() straight to s_validate_sid(). A userland create_sid() that throws makes ps_create_sid_user() return NULL, and ps_validate_sid_user() then reaches ZVAL_STR_COPY() with a NULL key. Reproducer is a SessionHandler subclass whose create_sid() throws on its second call, with validateId() defined, then session_create_id() on an active session: SIGSEGV on 8.4, 8.5 and master.

Every other s_create_sid() call site in ext/session already tests for NULL (php_session_initialize twice, session_regenerate_id twice, PS_READ_FUNC(mm)); this was the one that did not. It now stops the retry loop and propagates the pending exception rather than reporting a generic "Failed to create new ID", with the original throwable kept as the previous exception.

@iliaal
iliaal force-pushed the fix/session-create-id-null-sid branch 2 times, most recently from 873c330 to 61243b1 Compare July 29, 2026 18:00
session_create_id() passes the return value of s_create_sid() straight
into s_validate_sid(). A userland create_sid() that throws makes
ps_create_sid_user() return NULL, and ps_validate_sid_user() then reaches
ZVAL_STR_COPY() with a NULL key. Stop the retry loop when no ID was
created and propagate a pending exception instead of reporting a plain
failure. The four s_create_sid() call sites in php_session_initialize()
and session_regenerate_id() already test for NULL.

Closes phpGH-22924
@iliaal
iliaal force-pushed the fix/session-create-id-null-sid branch from 61243b1 to f9b3c7b Compare July 29, 2026 18:02
@iliaal

iliaal commented Jul 29, 2026

Copy link
Copy Markdown
Owner Author

Promoted upstream as php#22924.

@iliaal iliaal closed this Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant