[security][CVE-2019-9740][CVE-2019-9947] HTTP Header Injection (follow-up of CVE-2016-5699)

@gpshead

BPO	30458
Nosy	@gpshead, @jaraco, @vstinner, @larryhastings, @benjaminp, @ned-deily, @ambv, @vadmium, @serhiy-storchaka, @zhangyangyu, @stratakis, @orangetw, @miss-islington, @tirkarthi, @ware, @ret2libc
PRs	bpo-30458: Disallow control chars in http URLs. #12755 bpo-30458: Disable https related urllib tests on a build without ssl #13032 bpo-30458: Use InvalidURL instead of ValueError. #13044 [3.7] bpo-30458: Disallow control chars in http URLs. (GH-12755) #13154 [3.6] bpo-30458: Disallow control chars in http URLs. (GH-12755) #13155 [3.5] bpo-30458: Disallow control chars in http URLs. (GH-12755) #13207 [2.7] bpo-30458: Disallow control chars in http URLs. (GH-12755) (GH-13154) #13315 bpo-35906: Fix CRLF injection in urllib #12524 bpo-35906: Avoid headers injections in urllib #11768 bpo-30458: Disallow control chars in http URLs. (GH-12755) #13771 bpo-38216, bpo-36274: Allow subclasses to override validation and encoding behavior #16321

^{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:

assignee = None
closed_at = <Date 2019-12-09.03:10:03.575>
created_at = <Date 2017-05-24.15:01:31.731>
labels = ['type-security', 'deferred-blocker', '3.8', '3.9', '3.7', 'library']
title = '[security][CVE-2019-9740][CVE-2019-9947] HTTP Header Injection (follow-up of CVE-2016-5699)'
updated_at = <Date 2019-12-09.03:10:03.572>
user = 'https://github.com/orangetw'

bugs.python.org fields:

activity = <Date 2019-12-09.03:10:03.572>
actor = 'gregory.p.smith'
assignee = 'none'
closed = True
closed_date = <Date 2019-12-09.03:10:03.575>
closer = 'gregory.p.smith'
components = ['Library (Lib)']
creation = <Date 2017-05-24.15:01:31.731>
creator = 'orange'
dependencies = []
files = []
hgrepos = []
issue_num = 30458
keywords = ['patch', 'security_issue']
message_count = 55.0
messages = ['294360', '295026', '295067', '306981', '337970', '339754', '339840', '339846', '339848', '339850', '339851', '339852', '339853', '339857', '339858', '339861', '339884', '339894', '340405', '340407', '340408', '341174', '341175', '341176', '341178', '341192', '341234', '341286', '341290', '341291', '341724', '341750', '341906', '341932', '342470', '343045', '343104', '344826', '347282', '347285', '347290', '347897', '350003', '350028', '352451', '352596', '352727', '352731', '352751', '352760', '355246', '355261', '355298', '357988', '358050']
nosy_count = 16.0
nosy_names = ['gregory.p.smith', 'jaraco', 'vstinner', 'larry', 'benjamin.peterson', 'ned.deily', 'lukasz.langa', 'martin.panter', 'serhiy.storchaka', 'xiang.zhang', 'cstratak', 'orange', 'miss-islington', 'xtreak', 'ware', 'rschiron']
pr_nums = ['12755', '13032', '13044', '13154', '13155', '13207', '13315', '12524', '11768', '13771', '16321']
priority = 'deferred blocker'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue30458'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9']

Hi, the patch in CVE-2016-5699 can be broke by an addition space.
http://www.cvedetails.com/cve/CVE-2016-5699/
https://hg.python.org/cpython/rev/bf3e1c9b80e9
https://hg.python.org/cpython/rev/1c45047c5102

import urllib, urllib2

urllib.urlopen('http://127.0.0.1\r\n\x20hihi\r\n :11211')
urllib2.urlopen('http://127.0.0.1\r\n\x20hihi\r\n :11211')

Looking at the code and the previous issue bpo-22928, CRLF immediately followed by a tab or space (obs-fold: CRLF 1*( SP / HTAB )) is a valid part of a header value so the regex deliberately ignore them.

So it looks right to me the url given doesn't raise the same exception as the url without spaces, though the given url seems malformed.

You can also inject proper HTTP header fields (or do multiple requests) if you omit the space after the CRLF:

urlopen("http://localhost:8000/ HTTP/1.1\r\nHEADER: INJECTED\r\nIgnore:")

Data sent to the server:
>>> server = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)
>>> server.bind(("localhost", 8000))
>>> server.listen()
>>> [conn, addr] = server.accept()
>>> pprint(conn.recv(300).splitlines(keepends=True))
[b'GET / HTTP/1.1\r\n',
 b'HEADER: INJECTED\r\n',
 b'Ignore: HTTP/1.1\r\n',
 b'Accept-Encoding: identity\r\n',
 b'User-Agent: Python-urllib/3.5\r\n',
 b'Connection: close\r\n',
 b'Host: localhost:8000\r\n',
 b'\r\n']

bpo-14826 is already open about how “urlopen” handles spaces, and there is a patch in bpo-13359 that proposes to also encode newline characters. But if the CRLF or header injection is a security problem, then 2.7 etc could be changed to raise an exception (like bpo-22928), or to do percent encoding.

Actually, the CRLF + space can be injected via percent encoding, so just dealing with literal CRLFs and spaces wouldn’t be enough. You would have to validate the hostname after it is decoded.

urlopen("http://127.0.0.1%0D%0A%20SLAVEOF . . . :6379/")

>>> pprint(conn.recv(300).splitlines(keepends=True))
[b'GET / HTTP/1.1\r\n',
 b'Accept-Encoding: identity\r\n',
 b'Host: 127.0.0.1\r\n',
 b' SLAVEOF . . . :6379\r\n',
 b'Connection: close\r\n',
 b'User-Agent: Python-urllib/2.7\r\n',
 b'\r\n']

See also https://bugs.python.org/issue36276 for a similar report. I think it's better to raise an error instead of encoding CRLF characters in URL similar to headers.

I feel either of the issue and more preferably bpo-36276 closed as a duplicate of this one. Copy of msg337968 with reference to details about similar report in golang :

For reference an exact report on golang repo : golang/go#30794 . This seemed to have been fixed in latest golang release 1.12 and commit golang/go@829c5df . The commit introduces a check for CTL characters and throws an error for URLs something similar to Python does for headers now at bf3e1c9b80e9.

func isCTL(r rune) bool {
return r < ' ' || 0x7f <= r && r <= 0x9f
}

if strings.IndexFunc(ruri, isCTL) != -1 {
	return errors.New("net/http: can't write control character in Request.URL")
}

So below program used to work before go 1.12 setting a key on Redis but now it throws error :

package main

import "fmt"
import "net/http"

func main() {
resp, err := http.Get("http://127.0.0.1:6379?\\r\\nSET test failure12\r\n:8080/test/?test=a")
fmt.Println(resp)
fmt.Println(err)
}

➜ go version
go version go1.12 darwin/amd64
➜ go run urllib_vulnerability.go
<nil>
parse http://127.0.0.1:6379?
SET test failure12
:8080/test/?test=a: net/url: invalid control character in URL

Looking more into the commit there seemed to be a solution towards escaping characters with golang/go#22907 . The fix seemed to have broke Google's internal tests [0] and hence reverted to have the above commit where only CTL characters were checked and raises an error. I think this is a tricky bug upon reading code reviews in the golang repo that has around 2-3 reports with a fix committed to be reverted later for a more conservative fix and the issue was reopened to target go 1.13 .

Thanks a lot for the report @ragdoll.guo

[0] https://go-review.googlesource.com/c/go/+/159157/2#message-39c6be13a192bf760f6318ac641b432a6ab8fdc8

The CVE-2019-9740 has been assigned to the bpo-36276:

... which has been marked as a duplicate of this issue.

Martin claimed "Actually, the CRLF + space can be injected via percent encoding"

I am unable to reproduce that behavior using urllib.request.urlopen() or urllib.request.URLopener.open() in my master/3.8 tree.

Oh, I didn't recall that this issue (this class of security vulnerabilities) has a so old history. I found *A LOT* of similar open issues. Here are my notes. Maybe most open issues should be closed as duplicate of this one to clarify the status of urllib in Python? :-)

Emails:

Open issues:

2011, bpo-13359: "urllib2 doesn't escape spaces in http requests"
Not marked as a security issue.
2012, bpo-14826: "urlopen URL with unescaped space"
Fix using quote(self.__original, safe="%/:=&?~#+!$,;'@()*[]|")
... and the changed has then be reverted because it broke buildbots.
Still open.
2013, bpo-17322: "urllib.request add_header() currently allows trailing spaces (and other weird stuff)"
Not marked as a security issue.
2014, bpo-22928: "HTTP header injection in urrlib2/urllib/httplib/http.client (CVE-2016-5699)"
Marked as fixed, but user Orange explained in the first comment of in
bpo-30458 that the fix is incomplete.
2017, bpo-30458: "[CVE-2019-9740][security] CRLF Injection in httplib" (this issue)
2017, bpo-32085: "[Security] A New Era of SSRF - Exploiting URL Parser in Trending Programming Languages!"
2019, bpo-35906: "[CVE-2019-9947] Header Injection in urllib" (another CVE!)

Closed issues:

2004, bpo-918368: "urllib doesn't correct server returned urls" (urllib)
FIXED BY: commit 7c2867f
Fix: fullurl = quote(fullurl, safe="%/:=&?~#+!$,;'@()*[]")
2005, bpo-1353433: "Http redirection error in urllib2.py" (urllib2)
FIXED BY: commit ddb84d7
Fix: newurl = newurl.replace(' ', '%20')
2005, bpo-1153027: "http_error_302() crashes with 'HTTP/1.1 400 Bad Request"
FIXED BY: commit 690ce9b (Lib/urllib/request.py)
Fix: fullurl = quote(fullurl, safe="%/:=&?~#+!$,;'@()*[]")
bpo-29606: "urllib FTP protocol stream injection"
Duplicate of bpo-30119.
bpo-30119: "(ftplib) A remote attacker could possibly attack by containing the newline characters"
FIXED BY: commmit 8c2d4cf
Fix: reject "\r" and "\n" in FTP.putline() (Lib/ftplib.py)
bpo-36276: "[CVE-2019-9740] Python urllib CRLF injection vulnerability"
Closed as duplicate of bpo-30458

Rejected pull requests:

https://github.com/python/cpython/pull/1216/files
bpo-29606: Reject "\n" in ftp_open() of Lib/urllib/request.py
https://github.com/python/cpython/pull/2800/files
bpo-29606: Reject "\n" in ftp_open() and open_ftp() of Lib/urllib/request.py
https://github.com/python/cpython/pull/2301/files
bpo-30713: The splittype(), splitport() and splithost() functions of the
urllib.parse module now reject URLs which contain a newline character.
https://github.com/python/cpython/pull/2303/files
bpo-30713: The splittype(), splitport() and splithost() functions of the
urllib.parse module now reject URLs which contain a newline character, but
splittype() accepts newlines after the type.

2019, bpo-35906: "[CVE-2019-9947] Header Injection in urllib" (another CVE!)

Gregory P. Smith just marked bpo-35906 as a duplicate of this issue. Copy of his msg339842:

"""
my fix proposed in bpo-30458 fixes this issue.

i do not think this one deserved its own CVE; at least https://nvd.nist.gov/vuln/detail/CVE-2019-9947's current text also points to the other one.
"""

Until the status of CVE-2019-9947 is clarified, I added CVE-2019-9947 in the title of this issue to help to better track all CVEs :-)

Did someone contact the CVE organization to do something with CVE-2019-9947?

If I understand Jason's message correctly, the changes for bpo-30458 introduced a regression in 3.7.4 and will introduce the same regression in other branches as they are released, including 3.5.8 whose rc1 is now in testing. 3.7.5rc1 is scheduled to be tagged later today. Is this regression serious enough that we should hold 3.7.5 and/or 3.5.8 for a fix? If so, there should probably be a separate issue for it unless it is necessarily intertwined with the resolution of bpo-36274.

I'm provisionally setting the status of this issue to "release blocker".

Should we open a separate issue to track fixing the regression?

Should we open a separate issue to track (fixing) the regression?

Yes, I think so. The ticket I referenced mainly addresses an incompatibility that was introduced with Python 3.0, so is much less urgent than the one introduced more recently, so I believe it deserves a proper, independent description and discussion. I'll gladly file that ticket, tonight most likely.

I've created bpo-38216 to address the (perceived) regression.

With the breaking out of the portential and/or actual regression (e.g. invalid requests can no longer be crafted) into bpo-38216, itself a potential release blocker, we are still left here with the as-yet unresolved issue identified above in msg34728 (e.g. not checking for control characters in the "host" part of the URL, only the "path" part). Since this also affects so many branches/releases and has external components (CVE's, third-party impacts), it probably would have made sense to break it out into a separate issue (and maybe it still does). But since this problem has been present for many releases (apparently), I would rather not further hold the 3.7.5 release for a resolution (though that would be a good thing) so I'm going to change the priority for the moment to "deferred blocker".

But we need someone (preferably a core dev already involved) to take charge of this and push it to a resolution. Thanks for everyone's help so far!

CVE-2019-18348 has been assigned to the issue explained in https://bugs.python.org/issue30458#msg347282 . Maybe a separate bug for it would be better though. CVE-2019-18348 is about injecting CRLF in HTTP requests through the *host* part of a URL.

Can you please open a separate issue for CVE-2019-18348? It is easier to track that way.

(META: In general I think the CVE process is being abused and that these really did not deserve that treatment. https://lwn.net/Articles/801157/ is good reading and food for thought.)

I have created https://bugs.python.org/issue38576 to address CVE-2019-18348.

@gregory.p.smith if you have particular complains about these CVEs feel free to let me know (even privately). I think the security impact of these flaws is: an application that relies on urlopen/HTTPConnection/etc. where either the query part, the path part or the host part are user-controlled, could be exploited to send unintended HTTP headers to other hosts (maybe services that would not be directly reachable by the user).

FYI, there were some good replies to that CVE talk, one of which is https://grsecurity.net/reports_of_cves_death_greatly_exaggerated .

What is the status of this issue? Now that bpo-38576 has been opened to cover the host address part, can this issue be closed or downgraded? Should bpo-38576 be a release blocker?

i believe new work will be done via the new issue. marking this closed. if there is something not covered by bpo-38576 that remains, please open a new issue for it. new discussion on this long issue is easy to get lost in.

Navigation Menu

Repository navigation

[security][CVE-2019-9740][CVE-2019-9947] HTTP Header Injection (follow-up of CVE-2016-5699) #74643

52 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

[security][CVE-2019-9740][CVE-2019-9947] HTTP Header Injection (follow-up of CVE-2016-5699) #74643

Description

Activity

orangetw commented on May 24, 2017

zhangyangyu commented on Jun 2, 2017

vadmium commented on Jun 3, 2017

vadmium commented on Nov 26, 2017

tirkarthi commented on Mar 15, 2019

vstinner commented on Apr 9, 2019

gpshead commented on Apr 10, 2019

vstinner commented on Apr 10, 2019

vstinner commented on Apr 10, 2019

52 remaining items

ned-deily commented on Sep 17, 2019

larryhastings commented on Sep 18, 2019

jaraco commented on Sep 18, 2019

jaraco commented on Sep 18, 2019

ned-deily commented on Sep 18, 2019

ret2libc commented on Oct 23, 2019

gpshead commented on Oct 23, 2019

ret2libc commented on Oct 24, 2019

ned-deily commented on Dec 7, 2019

gpshead commented on Dec 9, 2019

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions