Opened 6 years ago

Closed 4 years ago

Last modified 4 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 4 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 6 years ago by Russell Keith-Magee

Triage Stage: UnreviewedAccepted

This should be enforced consistently.

comment:2 Changed 6 years ago by Aymeric Augustin

comment:3 Changed 5 years ago by Julien Phalip

Severity: Normal
Type: Cleanup/optimization

comment:4 Changed 5 years ago by Aymeric Augustin

In [17155]:

(The changeset message doesn't reference this ticket)

comment:5 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

Changed 4 years ago by Aymeric Augustin

Attachment: 13478.1.patch added

comment:7 Changed 4 years ago by Aymeric Augustin

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 Changed 4 years ago by Aymeric Augustin

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 Changed 4 years ago by Aymeric Augustin

In [17911]:

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

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