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

🔧 Reduce discoverability of session cookie name. #4305

Merged
merged 2 commits into from
Jun 29, 2017
Merged

🔧 Reduce discoverability of session cookie name. #4305

merged 2 commits into from
Jun 29, 2017

Conversation

coderabbi
Copy link
Contributor

Derives session.cookie from SESSION_COOKIE, falling back to (snake_cased) APP_NAME . '_session', falling back to 'laravel_session' (current) in order to make it less discoverable, thereby (slightly) reducing threat vector.

Derives session.cookie from SESSION_COOKIE, falling back to (snake_cased) APP_NAME . '_session', falling back to 'laravel_session' (current) in order to make it less discoverable, thereby (slightly) reducing threat vector.
@coderabbi coderabbi changed the title 🔧 🔧 Reduce discoverability of session cookie name. 🔧 Reduce discoverability of session cookie name. Jun 29, 2017
@taylorotwell
Copy link
Member

Could any characters be added to the app name which would in turn make this cookie name invalid (spaces, special characters, etc.?)

@coderabbi
Copy link
Contributor Author

In practice, no.

https://stackoverflow.com/questions/1969232/allowed-characters-in-cookies

Though perhaps it would be "good citizenship" to limit to those in RFC 6265 (http://www.ietf.org/rfc/rfc6265.txt), i.e. all alphanumeric plus !#$%&'*+-.^_`|~ are fair game.

I can strip all but those out, if you'd like, or not, as either approach is technically valid.

@GrahamCampbell
Copy link
Member

Don't we have a slug function that does that?

@coderabbi
Copy link
Contributor Author

Str::slug() is a bit more stringent that even RFC 6265 requires, but that would certainly work, yes (albeit with the interesting side effect that @ gets converted to 'at').

@coderabbi
Copy link
Contributor Author

Updated it to use str_slug, so is compliant with RFC 6265.

@taylorotwell taylorotwell merged commit 2871967 into laravel:develop Jun 29, 2017
@taylorotwell
Copy link
Member

Seems OK, Thanks.

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.

None yet

3 participants