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.

classification
Title: Security hole in wsgiref.headers.Headers
Type: security Stage: patch review
Components: Library (Lib) Versions: Python 3.9, Python 3.8, Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Arfrever, Felix.Gröbert, christian.heimes, devin, epicfaace, martin.panter, pje, vstinner
Priority: normal Keywords: patch

Created on 2011-03-25 12:14 by Felix.Gröbert, last changed 2022-04-11 14:57 by admin.

Files
File name Uploaded Description Edit
header_newlines_tip.patch devin, 2013-02-25 19:12 review
Pull Requests
URL Status Linked Edit
PR 15299 open epicfaace, 2019-08-15 04:09
Messages (10)
msg132080 - (view) Author: Felix Gröbert (Felix.Gröbert) Date: 2011-03-25 12:14
As noted by security@python.org's response I'm filing this bug here.


In wsgiref.headers.Headers it is possible to include headers which
contain a newline (i.e. \n or \r) either through add_header or
__init__. It is not uncommon that developers provide web applications
to the public in which the HTTP response headers are not filtered for
newlines but are controlled by the user. In such scenarios a malicious
user can use a newline to inject another header or even initiate a
HTTP response body. The impact would be at least equivalent to XSS.
Therefore, I suggest to filter/warn/except header tuples which contain
the above characters upon assignment in wsgiref.headers.
msg132132 - (view) Author: PJ Eby (pje) * (Python committer) Date: 2011-03-25 18:41
> It is not uncommon that developers provide web applications
to the public in which the HTTP response headers are not filtered for
newlines but are controlled by the user.

Really?  Which applications, and which response headers?

> Therefore, I suggest to filter/warn/except header tuples which contain
the above characters upon assignment in wsgiref.headers.

Applications that send them are not WSGI compliant anyway, since the spec forbids control characters in header strings -- and wsgiref.validate already validates this.

Still, I'm not aware of any legitimate use case for apps sending user input as an HTTP header where the data wouldn't already be escaped in some fashion -- cookies, URLs, ...?
msg132393 - (view) Author: Felix Gröbert (Felix.Gröbert) Date: 2011-03-28 11:23
If the spec forbids control characters in headers, the module should
enforce that.

The most frequent example of header injection is the redirect-case: an
application is forwarding using the Location header to a user-supplied
URL.
http://google.com/codesearch?as_q=self.redirect%5C%28self.request.get
Other examples are proxies, setting user-agent, or, as you mention,
custom set-cookies headers.
msg182766 - (view) Author: Devin Cook (devin) Date: 2013-02-23 17:00
Should now be compliant with this part of the spec:

"Each header_value must not include any control characters, including carriage returns or linefeeds, either embedded or at the end. (These requirements are to minimize the complexity of any parsing that must be performed by servers, gateways, and intermediate response processors that need to inspect or modify response headers.)"
msg182781 - (view) Author: Devin Cook (devin) Date: 2013-02-23 17:46
backported patch to 2.7
msg182782 - (view) Author: Devin Cook (devin) Date: 2013-02-23 17:51
backported patch to 2.6
msg182973 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-02-25 18:37
+        if bad_header_value_re.search(_value):
+            error_str = "Bad header value: {0!r} (bad char: {1!r})"
+            raise AssertionError(error_str.format(
+                _value, bad_header_value_re.search(_value).group(0)))

Why do you search the character twice? You can do something like:

match = bad_header_value_re.search(_value)
if match is not None:
  ... match..group(0) ...

Why do you only check value? You should also check _params:

parts = "; ".join(parts)
match = bad_header_value_re.search(parts)
...

And you should also check the name.

Should we do the same checks in httplib?
msg182976 - (view) Author: Devin Cook (devin) Date: 2013-02-25 19:12
The spec doesn't say anything about the header name. It probably should though, as the same issue exists there.

I used two searches because that's how it's done in wsgiref.validate, and it's not a huge deal to do that because the second one will only execute when there's an error. That said, I changed it to how you proposed.

Here's another stab at that patch.
msg195487 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-08-17 15:43
What do the RFCs for RFC-822 and HTTP 1.1 say about \r and \n in header names?
msg195491 - (view) Author: Devin Cook (devin) Date: 2013-08-17 16:14
It looks like it's allowed for header line continuation.

http://www.ietf.org/rfc/rfc2616.txt

HTTP/1.1 header field values can be folded onto multiple lines if the
continuation line begins with a space or horizontal tab. All linear
white space, including folding, has the same semantics as SP. A
recipient MAY replace any linear white space with a single SP before
interpreting the field value or forwarding the message downstream.

...

A CRLF is allowed in the definition of TEXT only as part of a header
field continuation. It is expected that the folding LWS will be
replaced with a single SP before interpretation of the TEXT value.
History
Date User Action Args
2022-04-11 14:57:15adminsetgithub: 55880
2019-08-15 04:22:13epicfaacesetnosy: + martin.panter
2019-08-15 04:09:21epicfaacesetstage: needs patch -> patch review
pull_requests: + pull_request15022
2019-08-15 04:03:30epicfaacesetnosy: + epicfaace

versions: + Python 3.8, Python 3.9
2017-11-25 21:34:11martin.panterlinkissue28778 dependencies
2016-09-08 23:44:52christian.heimessetassignee: pje ->
versions: + Python 3.5, Python 3.6, Python 3.7, - Python 2.6, Python 3.1, Python 3.2, Python 3.3
2013-08-18 22:58:15Arfreversetnosy: + Arfrever
2013-08-17 16:14:16devinsetmessages: + msg195491
2013-08-17 15:43:15christian.heimessetnosy: + christian.heimes
messages: + msg195487
2013-02-25 19:15:19devinsetfiles: - header_newlines_2.6.patch
2013-02-25 19:15:11devinsetfiles: - header_newlines_2.7.patch
2013-02-25 19:12:21devinsetfiles: + header_newlines_tip.patch

messages: + msg182976
2013-02-25 19:01:08devinsetfiles: - header_newlines.patch
2013-02-25 18:37:35vstinnersetnosy: + vstinner
messages: + msg182973
2013-02-23 17:51:37devinsetfiles: + header_newlines_2.6.patch

messages: + msg182782
2013-02-23 17:46:18devinsetfiles: + header_newlines_2.7.patch

messages: + msg182781
2013-02-23 17:00:48devinsetfiles: + header_newlines.patch

nosy: + devin
messages: + msg182766

keywords: + patch
2011-06-01 06:30:00terry.reedysetversions: - Python 2.5
2011-03-28 11:23:32Felix.Gröbertsetmessages: + msg132393
2011-03-25 18:41:27pjesetmessages: + msg132132
2011-03-25 16:09:44eric.araujosetnosy: + pje
title: Potential misuse of wsgiref.headers.Headers -> Security hole in wsgiref.headers.Headers
assignee: pje
versions: + Python 2.6, Python 2.5, Python 3.1, Python 2.7, Python 3.2
stage: needs patch
2011-03-25 12:14:57Felix.Gröbertcreate