[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Change by Ashwin Ramaswami : -- pull_requests: +15023 pull_request: https://github.com/python/cpython/pull/15299 ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by Ned Deily : -- assignee: georg.brandl -> priority: release blocker -> resolution: -> fixed stage: backport needed -> resolved status: open -> closed ___ Python tracker

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Ned Deily added the comment: New changeset 8e88f6b5e2a35ee458c161aa3f2b7f1f17fb45d1 by Ned Deily (Serhiy Storchaka) in branch '3.3': [3.3] bpo-22928: Disabled HTTP header injections in http.client. (#2817) https://github.com/python/cpython/commit/8e88f6b5e2a35ee458c161aa3f2b7f1f17fb45d1

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Serhiy Storchaka added the comment: \A is not needed. match() always matches from the start. -- ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

STINNER Victor added the comment: > What is the difference between PR 2817 and PR 2861? Oh crap, I didn't know that you already created a PR. I compared the two PR: * My PR adds \A at the start of: _is_legal_header_name = re.compile(rb'\A[^:\s][^:\r\n]*\Z').match * My PR uses blurb, yours

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Serhiy Storchaka added the comment: What is the difference between PR 2817 and PR 2861? -- ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by STINNER Victor : -- pull_requests: +2912 ___ Python tracker ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by Serhiy Storchaka : -- nosy: +benjamin.peterson, larry priority: normal -> release blocker ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by Serhiy Storchaka : -- pull_requests: +2870 ___ Python tracker ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by Kubilay Kocak : -- stage: resolved -> backport needed ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Ned Deily added the comment: Getting to be the last chance to backport this for 3.3.x. -- nosy: +ned.deily ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by Serhiy Storchaka : -- assignee: serhiy.storchaka -> georg.brandl ___ Python tracker ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

Changes by koobs : -- versions: +Python 3.3 ___ Python tracker ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)

koobs added the comment: 3.3 is supported for security related fixes until September 2017 [1], but only 3.4, 3.5 and 2.7 have received the backport, reopen for outstanding merge [1] https://docs.python.org/devguide/#status-of-python-branches Update summary to reflect the RedHat CVE that was

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Serhiy Storchaka : -- nosy: +georg.brandl ___ Python tracker ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Vlad K. added the comment: Doesn't this affect Python 3.3 as well, which is in security-only mode? Shouldn't that version be patched as well? -- nosy: +vladk ___ Python tracker

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Serhiy Storchaka added the comment: Added new tests and tweaked regexpes. Thank you for your contribution Demian. Now we can start long-standing deprecation process for conforming to RFC. -- resolution: - fixed stage: commit review - resolved status: open - closed versions: +Python

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Roundup Robot added the comment: New changeset 1c45047c5102 by Serhiy Storchaka in branch '2.7': Issue #22928: Disabled HTTP header injections in httplib. https://hg.python.org/cpython/rev/1c45047c5102 New changeset bf3e1c9b80e9 by Serhiy Storchaka in branch '3.4': Issue #22928: Disabled HTTP

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: Thanks for the tweaks Serhiy, those seem reasonable to me. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Serhiy Storchaka added the comment: I'll return previous implementation of header value regex. All other LGTM. -- stage: patch review - commit review ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Demian Brecht demianbre...@gmail.com: Added file: http://bugs.python.org/file38449/issue22928_6.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: I much prefer the new patch with better compatibility and flexibility :) If you want to strengthen the tests to reflect some of the decisions made here you could add the following tests: Positive tests: * putheader('C1-Control', b'next\x85line') *

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: Latest patch should now address all comments. -- Added file: http://bugs.python.org/file38433/issue22928_5.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Serhiy Storchaka added the comment: May be not drop folding support, but just deprecate the putheader() multi-argument mode? And of course add sanity checks for separate putheader() arguments. -- ___ Python tracker rep...@bugs.python.org

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: After a chat with David and getting my head wrapped more around backwards compatibility, I also agree that the changes in the patch are far too strict. It's much more important to preserve backwards compatibility than to strictly conform to the RFC. I've

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Serhiy Storchaka added the comment: Added comments on Rietveld. Is there a limit to the length of header line? Would not unfolding all header values exceed the limit? -- assignee: - serhiy.storchaka nosy: +serhiy.storchaka ___ Python tracker

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: Folded header fields are deprecated as of RFC 7230; see https://tools.ietf.org/html/rfc7230#section-3.2.4. The only reasons to fold them I can think of is for readability (debugging), when generating a messsage/http MIME message (which I don’t think the Python

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Berker Peksag berker.pek...@gmail.com: -- nosy: +berker.peksag stage: - patch review versions: -Python 3.6 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Demian Brecht demianbre...@gmail.com: Added file: http://bugs.python.org/file38190/issue22928_3.patch ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: I’ve updated the patch to include the latin-1 charset in legal header values. It still uses a space as delimiter, but all other comments should now be addressed. -- Added file: http://bugs.python.org/file38158/issue22928_2.patch

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: Maybe join them with tabs rather than spaces then, since it was previously \r\n\t. This way it is even closer to before. After thinking about this a little more, I think I'd prefer to keep spaces rather than tabs. The reason being is that, in my mind, now

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Demian Brecht demianbre...@gmail.com: -- versions: -Python 2.7, Python 3.2, Python 3.3, Python 3.4 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: But it is not natural to do things like this (based on headers sent by Firefox): putheader(User-Agent, Mozilla/5.0, (X11;, Linux, x86_64;, rv:25.0), Gecko/20100101, Firefox/25.0) putheader(Accept-Encoding, gzip,, deflate) A way to properly encode different

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: But it is not natural to do things like this (based on headers sent by Firefox) Good point. Otherwise, retaining the one_value.encode('latin-1') call is confusing when later on it rejects non-ASCII-encoded characters. I’m a little torn on this one given

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: Here's a patch addressing the potential vulnerability as reported. The patch should also bring the implementation up to date with the most recent standards around header names and values. There could be potential for breaking compatibility if people are

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: If we’re in the realm of 3.5 only changes, it might make sense to remove the multi-argument mode of putheader() altogether, and document it only generates a single line. (Currently still says it generates multiple lines.) --

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Demian Brecht added the comment: I think that keeping the public API as-is is the better way to go, at least for the shorter term given it won't require users to have to make code changes. Thanks for the catch on the docs though, will update that. --

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: Good point. Maybe join them with tabs rather than spaces then, since it was previously \r\n\t. This way it is even closer to before. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Martin Panter added the comment: There could be potential for breaking compatibility if people are intentionally sending values with folded lines (obsoleted by the new HTTP RFC). Perhaps the same error should be raised for values that cannot be encoded in Latin-1? Also, maybe most control

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by Demian Brecht demianbre...@gmail.com: -- nosy: +demian.brecht ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

Changes by R. David Murray rdmur...@bitdance.com: -- nosy: +orsenthil, r.david.murray ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue22928 ___ ___

[issue22928] HTTP header injection in urrlib2/urllib/httplib/http.client

New submission from Guido Vranken: Proof of concept: # Script for Python 2 import urllib2 opener = urllib2.build_opener() opener.addheaders = [('User-agent', 'Mozilla/5.0' + chr(0x0A) + Location: header injection)] response = opener.open(http://localhost:;) # Data sent is: GET / HTTP/1.1