#24915 closed Cleanup/optimization (fixed)
Stricter validation on session key
| Reported by: | Sasha Romijn | Owned by: | David Bannon |
|---|---|---|---|
| Component: | contrib.sessions | Version: | 1.8 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Ready for checkin | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
Recently we had a vulnerability where session keys were set to empty strings: https://www.djangoproject.com/weblog/2015/may/20/security-release/
I can't really imagine any case where setting empty strings as session keys is a sensible thing to do. I therefore think we should add some basic validation on the key. Perhaps we should have a minimum length of 5-8 characters, because it would be just as problematic if it were only one or two characters. This doesn't make it impossible to have weak session keys, but it is a very basic hardening that would protect us from such a bug in the future.
Without having looked at the code, my first idea is that this belongs in the session backends somewhere. This breaks backwards compatibility, but given the rationale I think a mention in the release notes is sufficient.
Change History (8)
comment:1 by , 10 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|
comment:2 by , 10 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:3 by , 10 years ago
| Has patch: | set |
|---|
comment:4 by , 10 years ago
| Triage Stage: | Accepted → Ready for checkin |
|---|
comment:6 by , 10 years ago
@sp1ky: thanks for the patch. This patch might qualify under the Google Patch Rewards programme: http://www.google.com/about/appsecurity/patch-rewards/
As the patch author, you can try to submit it.
A patch only qualifies once it has been shipped in a release, so you will have to wait with your submission until 1.9 is actually released. If you do submit, I'd make sure to mention the blogpost about the vulnerability, as it shows this is actually a real potential problem that we've now, at least partially, tackled for the future.
follow-up: 8 comment:7 by , 10 years ago
In my experience you don't need to wait until the release ships to submit to Google.
comment:8 by , 10 years ago
Cool, thanks for the tip @timgraham, @erikr. I'll fire a message off to the Patch Rewards program and see what happens!
https://github.com/django/django/pull/4807
Changed _session_key attribute to a property and implemented basic
validation in the setter. The session key must be 'truthy' and
at least 8 characters long. Otherwise, the value is set to None