This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author martin.panter
Recipients Pathangi Jatinshravan, Tim.Graham, gvanrossum, martin.panter, pitrou, r.david.murray
Date 2015-11-01.06:11:42
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1446358303.32.0.47944550947.issue25228@psf.upfronthosting.co.za>
In-reply-to
Content
Adding Guido and Antoine, who committed the security fix as 9e765e65e5cb in 2.7 and 5cfe74a9bfa4 in 3.2. Perhaps you are able to help decide if the proposal here would affect the original security report. Basically this issue (as well as #22758 and #22983) are complaining that 3.2’s cookie parsing became too strict. People would like to parse subsequent cookie “morsels” if an earlier one is considered invalid, rather than aborting the parse completely.

All I can find out about the security report is from <https://bugs.python.org/issue22796#msg230650> and <https://hackerone.com/reports/14883>, but that doesn’t explain the test cases with square brackets in the cookie names.

RFC 6265 says double quotes (") are not meant to be sent by the server, but the client should tolerate them without any special handling (different to Python’s handling and earlier specs, which parse a special double-quoted string syntax). One potential problem that comes to mind is that the current patch blindly searches for the next semicolon “;”, which would not be valid inside a double-quoted string, e.g. name="some;value".

Python behaviour:

* Before the 3.2 security fix, square brackets and double quotes caused truncation of the cookie value, but subsequent cookies were still parsed in most cases

* The security fix prevents parsing of subsequent cookies (either on purpose or as a side effect)

* The HttpOnly and Secure support in 3.3+ (Issue 16611) prevents parsing of the cookie morsel with the offending square bracket or double quote. This is proposed for 3.2 backport in Issue 22758.

* Square brackets are now allowed in 3.2+ thanks to Issue 22931. So 3.2 should truncate the original test case at the double quote, while 3.3+ drops the offending cookie.

The current patch proposed here appears to solve Issue 22983 (permissive parsing) in general. If the current cookie does not match the syntax, it is skipped, by falling back to a search for a semicolon “;”. So I am inclined to close Issue 22983 as a duplicate of this issue.

And Tim, I understand your main interest in Issue 22758 is that parsing aborts for things like "a=value1; HttpOnly; b=value2". If this patch were ported to 3.2 it should also fix that for free.

Pathangi: did you see my review comment about unnecessary backslashes? I also left another comment today.
History
Date User Action Args
2015-11-01 06:11:43martin.pantersetrecipients: + martin.panter, gvanrossum, pitrou, r.david.murray, Tim.Graham, Pathangi Jatinshravan
2015-11-01 06:11:43martin.pantersetmessageid: <1446358303.32.0.47944550947.issue25228@psf.upfronthosting.co.za>
2015-11-01 06:11:43martin.panterlinkissue25228 messages
2015-11-01 06:11:42martin.pantercreate