[issue25228] Regression in cookie parsing with brackets and quotes

Collin Anderson added the comment: It should be safe to hard split on semicolon. `name="some;value"` is not valid, even though it's quoted. I think raw double quotes, commas, semicolons and backslashes are _always_ invalid characters in cookie values. >From

[issue25228] Regression in cookie parsing with brackets and quotes

Collin Anderson added the comment: The issue I'm currently running into, is that although browsers correctly ignore invalid Set-Cookie values, they allow 'any CHAR except CTLs or ";"' in cookie values set via document.cookie. So, if you say document.cookie = 'key=va"lue; path=/', the browser

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: To move forward on this, I would like someone else (hopefully Antoine? :) to confirm my theory about the cookie injection attack, or otherwise explain why the patch won’t (re)open any security holes. Also, I would like to add some more test cases based on

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: Looking at this a second time, I think I have figured out what the security report was about. Before the fix (before revision 270f61ec1157), an attacker could trick the parser into accepting a separate key=value cookie “morsel”, when it was supposed to be part

[issue25228] Regression in cookie parsing with brackets and quotes

Changes by Guido van Rossum : -- nosy: -gvanrossum ___ Python tracker ___ ___

[issue25228] Regression in cookie parsing with brackets and quotes

Changes by Will Harris : -- nosy: +harris ___ Python tracker ___ ___ Python-bugs-list mailing

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: 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

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Just saw the code review comments now, didn't know that there was a separate section for code review comments until now. Will take a look and implement them. -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: New patch with code review comments incorporated. -- Added file: http://bugs.python.org/file40919/patch_review.diff ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Guido van Rossum added the comment: I'm coming at this without much context (I don't recall the original issue) but IIUC from a security POV, lenient parsing is unsafe -- it could allow an attacker to modify a cookie (or part of a cookie -- I'm unclear on the correct terminology here) and that's

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Has there been any movement on this issue? -- ___ Python tracker ___

[issue25228] Regression in cookie parsing with brackets and quotes

R. David Murray added the comment: Yes, we should get signoff from someone who was involved in the original security fix, since it was a security fix. -- nosy: +r.david.murray ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Hi, I've made the change to use str.find() and removed the while loop, can you take a look at it? -- Added file: http://bugs.python.org/file40717/patch.diff ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: The str.find() call was kind of what I had in mind. But I don’t feel qualified to say whether the fix is good in general. I would have to find out about at the Cookie header format, and understand what the security implications are to do with lax parsing.

[issue25228] Regression in cookie parsing with brackets and quotes

Tim Graham added the comment: Yes, when I have some time. By the way, did you intentionally remove all the "Python 3.X" versions on the issue? -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: Instead of the while loop, can’t you use something like str.find(";", i)? -- stage: needs patch -> patch review ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Hi Tim, I have submitted a patch (patch_with_test.diff). Can you take a look at this? -- ___ Python tracker ___

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Oh not intentional. Must have clicked something by mistake -- versions: +Python 3.2, Python 3.3, Python 3.4, Python 3.5, Python 3.6 ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Is this what you wanted? -- Added file: http://bugs.python.org/file40709/patch_with_test.diff ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Tim Graham added the comment: I had already proposed a test, see cookie-bracket-quotes-test.diff. What I meant was that the fix and the test should be combined into a single patch. -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Hi I have made a patch for this, can anyone review and let me know? -- versions: -Python 3.2, Python 3.3, Python 3.4, Python 3.5, Python 3.6 Added file: http://bugs.python.org/file40700/patch.diff ___ Python

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Hi Tim, I have submitted a patch for this issue (patch_final.diff, the earlier one failed a UT). Now all UTs are passing. Can you take a look at this? -- Added file: http://bugs.python.org/file40701/patch_final.diff

[issue25228] Regression in cookie parsing with brackets and quotes

Tim Graham added the comment: Could you please integrate my unit test into your patch? You also need to sign the PSF Contributor Agreement: https://www.python.org/psf/contrib/contrib-form/ -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Oops, sorry looks like a unit test is failing. I will fix it and submit another one soon. -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Added a patch where unit test has been modified to include the above case. I have signed the agreement. -- Added file: http://bugs.python.org/file40703/patch_unittest.diff ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: This is the first ever bug I will be working on so there might be a bit of a learning curve, but I'll do my best to come out with something by this week. -- ___ Python tracker

[issue25228] Regression in cookie parsing with brackets and quotes

Tim Graham added the comment: Sure, feel free to propose a patch. -- ___ Python tracker ___ ___

[issue25228] Regression in cookie parsing with brackets and quotes

Pathangi Jatinshravan added the comment: Hi, can I be assigned to this behaviour issue? -- nosy: +Pathangi Jatinshravan ___ Python tracker ___

[issue25228] Regression in cookie parsing with brackets and quotes

Tim Graham added the comment: It might be a case of issue22983. I'll try to look into the details and offer a patch next week. For what it's worth, there are other regressions in Python 3.2 cookie parsing that makes the latest patch release (3.2.6) unusable with Django (issue22758), so from

[issue25228] Regression in cookie parsing with brackets and quotes

Martin Panter added the comment: Thanks for the test case. It looks like the commit in question was done as a security fix in 3.2.6 and 3.3.6. I’m not sure on the policy, but maybe that justifies putting any fixes into 3.2+. I’m not familiar with HTTP cookies. Is this a case of a 100%

[issue25228] Regression in cookie parsing with brackets and quotes

New submission from Tim Graham: Regression in https://hg.python.org/cpython/rev/9e765e65e5cb (affects 2.7 and 3.2+), similar to issue22931 where inserting an invalid cookie value can cause the rest of the cookie to be ignored. A test is attached, and here's a quick demo: Old: >>> from