python / cpython

The Python programming language
https://www.python.org
Other
63.72k stars 30.53k forks source link

Bogus cookie generated after invalid cookie attribute is input #89521

Open 238efebd-8c46-41dc-b070-b1689c002a37 opened 3 years ago

238efebd-8c46-41dc-b070-b1689c002a37 commented 3 years ago
BPO 45358
Nosy @glubsy
PRs
  • python/cpython#28726
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = None closed_at = None created_at = labels = ['type-bug', 'library', '3.11'] title = 'Bogus cookie generated after invalid cookie attribute is input' updated_at = user = 'https://github.com/glubsy' ``` bugs.python.org fields: ```python activity = actor = 'greob' assignee = 'none' closed = False closed_date = None closer = None components = ['Library (Lib)'] creation = creator = 'greob' dependencies = [] files = [] hgrepos = [] issue_num = 45358 keywords = ['patch'] message_count = 1.0 messages = ['403114'] nosy_count = 1.0 nosy_names = ['greob'] pr_nums = ['28726'] priority = 'normal' resolution = None stage = 'patch review' status = 'open' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue45358' versions = ['Python 3.11'] ```

    238efebd-8c46-41dc-b070-b1689c002a37 commented 3 years ago

    Youtube sends cookies with some non-standard attributes. For example:

    Secure-1PSID=XXXXXX; Domain=.youtube.com; Path=/; Expires=Tue, 03-Oct-2023 16:26:27 GMT; Secure; HttpOnly; Priority=HIGH; SameParty

    Notice the Priority and SameParty attributes.

    In the case above, the cookie is entirely discarded because of the unexpected SameParty attribute. I have not read the specifications, but I would prefer to keep the cookie instead of discarding it. These unusual attributes are clearly used by Chromium. Firefox ignore these attributes and does not discard the cookies.

    In another case, the "Priority" key/value attribute is present, which may or may not be followed by any other (valid) attributes. An extra cookie is then generated by http.cookies.BaseCookie.__parse_string() (cpython/Lib/http/cookies.py:539):

    Set-Cookie: priority=high; Domain=www.youtube.com; Path=/; SameSite=none

    There may even be duplicate cookies generated if the case changes (ie. "Priority=HIGH" would be yet another bogus cookie).

    The reason for this is as follows: The "priority=high" is seen as a key/value pair, and added to the parsed_items list with type TYPEKEYVALUE, which is now the _second TYPEKEYVALUE in the list. To my understanding, there should be only _one TYPE_KEYVALUE in this list, that is the actual cookie key/value pair. Any other item added to that list should be a TYPE_ATTRIBUTE.

    In the for loop below (cpython/Lib/http/cookies.py:590), a new Morsel is created with key=Priority and value=HIGH, which is not what we want at all.

    I have been working on a patch, but I keep pulling my hair over the fact that multiple key=value pairs can be found in the same string, which is expected by the test suite to result in multiple separate cookies.

    An easy workaround would be to justadd "priority" to _reserved keys, and "sameparty" to known flags. Basically catching up with Google's "extensions".

    Thoughts?