Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#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 Robert Johnstone, 9 years ago

Triage Stage: UnreviewedAccepted

comment:2 by David Bannon, 9 years ago

Owner: changed from nobody to David Bannon
Status: newassigned

comment:3 by David Bannon, 9 years ago

Has patch: set

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

comment:4 by Carl Meyer, 9 years ago

Triage Stage: AcceptedReady for checkin

comment:5 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: assignedclosed

In f4416b1:

Fixed #24915 -- Added stricter session key validation

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.

comment:6 by Sasha Romijn, 9 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.

comment:7 by Tim Graham, 9 years ago

In my experience you don't need to wait until the release ships to submit to Google.

in reply to:  7 comment:8 by David Bannon, 9 years ago

Cool, thanks for the tip @timgraham, @erikr. I'll fire a message off to the Patch Rewards program and see what happens!

Note: See TracTickets for help on using tickets.
Back to Top