Opened 14 years ago

Closed 12 years ago

Last modified 12 years ago

#13478 closed Cleanup/optimization (needsinfo)

Session backends should all refuse user-defined, non-existant IDs

Reported by: Jeremy Dunck Owned by: nobody
Component: contrib.sessions Version: 1.1
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

DB session store refuses to use non-existant user-supplied session IDs. This is done in an attempt to avoid session fixation attacks.

Per George Sakkis on the mailing list, not all backends similarly refuse user-supplied IDs. File session apparently doesn't, for example.

All backends should be the same in this enforcement (or not).

Attachments (1)

13478.1.patch (1.3 KB ) - added by Aymeric Augustin 12 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

This should be enforced consistently.

comment:3 by Julien Phalip, 13 years ago

Severity: Normal
Type: Cleanup/optimization

comment:4 by Aymeric Augustin, 12 years ago

In [17155]:

(The changeset message doesn't reference this ticket)

comment:5 by Aymeric Augustin, 12 years ago

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 by Aymeric Augustin, 12 years ago

Easy pickings: unset

Change Easy pickings from NULL to False.

by Aymeric Augustin, 12 years ago

Attachment: 13478.1.patch added

comment:7 by Aymeric Augustin, 12 years ago

Has patch: set
Needs tests: set

Here's a patch for the docs, I also intend to add test cases validating that a non-existing session key is regenerated.

comment:8 by Aymeric Augustin, 12 years ago

Needs tests: unset
Resolution: needsinfo
Status: newclosed

The assertion on the mailing list was: "By the way, this does not apply to all backends; file SessionStore for example uses passed ids as is."

But it wasn't backed by any code, and we have a test proving the contrary: SessionTestsMixin.test_invalid_key.

I checked the code of the file-based session store, and it clearly creates a new session if there's no file matching the given session key.

It seems to me that this sentence was just hand waving.

(I'm going to commit the docs patch which is related to this ticket anyway.)

comment:9 by Aymeric Augustin, 12 years ago

In [17911]:

Clarified that Django randomizes session keys. Refs #11555, #13478, #18128.

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