classification
Title: http.cookies.SimpleCookie doesn't parse comma-only separated cookies correctly
Type: enhancement Stage: patch review
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Kyle Graehl, demian.brecht, martin.panter, remi.lapeyre, riklaunim
Priority: normal Keywords: patch

Created on 2015-04-13 13:17 by riklaunim, last changed 2019-01-26 12:15 by martin.panter.

Pull Requests
URL Status Linked Edit
PR 10494 open remi.lapeyre, 2018-11-12 22:03
Messages (4)
msg240603 - (view) Author: Piotr (riklaunim) Date: 2015-04-13 13:17
Skype WISPr and iPassConnect (and maybe other bots) return cookies as a comma separated list. It's not a comma + space (which works).

C = cookies.SimpleCookie()
C.load('a=b,z=zz')
>>> C['a']
<Morsel: a='b,z=zz'>

I wonder what would those bots do if there was a comma in a cookie value.
msg253356 - (view) Author: Kyle Graehl (Kyle Graehl) Date: 2015-10-22 22:24
r"(\s+|;|$|,)"   # Ending either at space, semicolon, or EOS. ( or comma...)

I remember running into this same problem like 5 years ago.

I added a comma as a valid regexp for ending the pattern, and removed it as a valid _LegalKeyChars

I also think adding the "Priority" reserved key might make sense (or at least have options for handling it)
msg329621 - (view) Author: Rémi Lapeyre (remi.lapeyre) * Date: 2018-11-10 15:27
This is not a valid cookie string and I think neither Django nor Nginx would understand this cookie correctly.

On the other hand, per RFC 6265 the comma is a forbidden character in a cookie value (https://tools.ietf.org/html/rfc6265#section-4.1.1):
    
    cookie-pair       = cookie-name "=" cookie-value
    cookie-name       = token
    cookie-value      = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE )
    cookie-octet      = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
                       ; US-ASCII characters excluding CTLs,
                       ; whitespace DQUOTE, comma, semicolon,
                       ; and backslash

so there is no official way to parse the given string (when a comma is present in the value, the cookie should be encoded as base 64).

Since this is not a valid cookie string anyway, I think the solution proposed by Kyle is appropriate.
msg334389 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2019-01-26 12:15
I think making a comma start a new cookie is dangerous, and perhaps this proposal should be rejected.

I’m not an expert on web programming, but this reminds me of some security problems that already affected Python: <https://translate.google.com/translate?u=https://habr.com/en/post/272187/>. In a web page, Java Script could set a cookie with a single name and a comma in the value.

document.cookie = 'a=b,csrftoken=INJECTED'

Currently, Python in the server would parse that the way the script intended:

>>> C = BaseCookie('a=b,csrftoken=INJECTED')
>>> C['a'].value
'b,csrftoken=INJECTED'
>>> C['csrftoken'].value
KeyError: 'csrftoken'

But with the proposed change, Python would be tricked into parsing it as two separate “morsels”:

>>> C['csrftoken'].value
'INJECTED'
History
Date User Action Args
2019-01-26 12:15:10martin.pantersettype: behavior -> enhancement

messages: + msg334389
nosy: + martin.panter
2018-11-12 22:03:36remi.lapeyresetkeywords: + patch
stage: patch review
pull_requests: + pull_request9752
2018-11-10 15:27:32remi.lapeyresetmessages: + msg329621
2018-11-10 10:29:13remi.lapeyresetnosy: + remi.lapeyre
2016-08-22 12:40:14martin.pantersettitle: SimpleCookie doesn't parse comma-only separated cookies correctly -> http.cookies.SimpleCookie doesn't parse comma-only separated cookies correctly
2015-10-22 22:24:30Kyle Graehlsetnosy: + Kyle Graehl
messages: + msg253356
2015-04-17 16:41:15demian.brechtsetnosy: + demian.brecht
2015-04-13 13:17:33riklaunimcreate