Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#13478 closed Cleanup/optimization (needsinfo)

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

Reported by: jdunck 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 aaugustin 3 years ago.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 5 years ago by russellm

  • Triage Stage changed from Unreviewed to Accepted

This should be enforced consistently.

comment:3 Changed 4 years ago by julien

  • Severity set to Normal
  • Type set to Cleanup/optimization

comment:4 Changed 3 years ago by aaugustin

In [17155]:

(The changeset message doesn't reference this ticket)

comment:5 Changed 3 years ago by aaugustin

  • UI/UX unset

Change UI/UX from NULL to False.

comment:6 Changed 3 years ago by aaugustin

  • Easy pickings unset

Change Easy pickings from NULL to False.

Changed 3 years ago by aaugustin

comment:7 Changed 3 years ago by aaugustin

  • 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 3 years ago by aaugustin

  • Needs tests unset
  • Resolution set to needsinfo
  • Status changed from new to closed

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 3 years ago by aaugustin

In [17911]:

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

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