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

[orangetw]

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']

[orangetw]

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')

[zhangyangyu]

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.

[vadmium]

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.

[vadmium]

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']

[tirkarthi]

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

[vstinner]

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

• https://nvd.nist.gov/vuln/detail/CVE-2019-9740
• https://bugzilla.redhat.com/show_bug.cgi?id=1692984

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

[gpshead]

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.

[vstinner]

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:

• 2019: https://mail.python.org/pipermail/python-dev/2019-April/157014.html
• 2017: https://mail.python.org/pipermail/python-dev/2017-July/148699.html

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.

[vstinner]

• 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?