Skip to content
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

[9.x] JSON Session Option #40595

Merged
merged 1 commit into from
Jan 25, 2022
Merged

[9.x] JSON Session Option #40595

merged 1 commit into from
Jan 25, 2022

Conversation

taylorotwell
Copy link
Member

This pull requests allows developers to choose to serialize session data in storage using PHP or JSON. Over the years, several people have expressed concerns with deserializing sessions using PHP's unserialize function, as this could lead to security problems (including remote code execution) if the application's encryption key is leaked.

However, all known ways to exploit this require the attacker to know the application's encryption key, which if known, could already be used to craft authenticated cookies for the application, etc. Nevertheless, offering this an option to those who would prefer to use JSON to serialize sessions in storage.

@taylorotwell taylorotwell changed the base branch from 8.x to 9.x January 24, 2022 21:54
@GrahamCampbell GrahamCampbell changed the title JSON Session Option [9.x] JSON Session Option Jan 24, 2022
@Krisell
Copy link
Contributor

Krisell commented Jan 25, 2022

This is very interesting, both from a security perspective and since it makes the payload around 10–15 % smaller (measured both on a clean install and on a larger project).

I am running an app which uses the cookie driver and I am probably going to switch to the json setting if/when merged. However I am going to have to do the migration transparently, meaning that no session data should be lost. That is achievable by overriding readFromHandler() to fallback to unserialize in case json_decode returns null. If/when this is merged and I can successfully verify that transparent migration, I might publish a tiny package, or just a gist, for that code (which only needs to run for just longer than one session lifetime). It could of course also be solved with a php-to-json setting in the core, but that is perhaps unnecessary for the narrow use case.

Edit: Package for migrating serialization transparently here:
https://github.com/Krisell/laravel-session-migrator/

: new Store(
$this->config->get('session.cookie'),
$handler,
$id = null,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this assignment just to clarify the parameter name? If so, what about named parameters instead?

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.

2 participants