Skip to content

Check session_create_id() input for null byte #14728

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 6, 2024

Conversation

jorgsowa
Copy link
Contributor

The function session_create_id() allows only selected characters amongst which there is no NULL byte, however the validation doesn't check for it.

Warning: session_create_id(): Prefix cannot contain special characters. Only the A-Z, a-z, 0-9, "-", and "," characters are allowed

This PR checks for a NULL byte in the function php_session_valid_key used by the PHP function session_create_id().

Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is sensible, the current implementation can be improved to take care of the issue better.

@jorgsowa jorgsowa requested a review from Girgias June 29, 2024 22:25
@jorgsowa jorgsowa requested a review from Girgias June 30, 2024 17:33
@jorgsowa jorgsowa force-pushed the session_id_null_byte branch from 3841447 to 844856c Compare July 3, 2024 22:28
@Girgias
Copy link
Member

Girgias commented Jul 4, 2024

I'd like to see a test where you use ini_set() to amend the session.name INI setting and have a nul byte in it.

@jorgsowa
Copy link
Contributor Author

jorgsowa commented Jul 4, 2024

@Girgias, I want to cover it in the next PRs. Function session_name() doesn't share the same state with session_create_id().

@Girgias Girgias merged commit 8e1561c into php:master Jul 6, 2024
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants