blob: 066ac68290e5bb2826f18121ef7509947bd572e2 [file] [log] [blame]
Brad Bishop96ff1982019-08-19 13:50:42 -04001From bb8071a4cae5ab3fe321481dd3d73662ffb26052 Mon Sep 17 00:00:00 2001
2From: Victor Stinner <victor.stinner@gmail.com>
3Date: Tue, 21 May 2019 15:12:33 +0200
4Subject: [PATCH] bpo-30458: Disallow control chars in http URLs (GH-12755)
5 (GH-13154) (GH-13315)
6MIME-Version: 1.0
7Content-Type: text/plain; charset=UTF-8
8Content-Transfer-Encoding: 8bit
9
10Disallow control chars in http URLs in urllib2.urlopen. This
11addresses a potential security problem for applications that do not
12sanity check their URLs where http request headers could be injected.
13
14Disable https related urllib tests on a build without ssl (GH-13032)
15These tests require an SSL enabled build. Skip these tests when
16python is built without SSL to fix test failures.
17
18Use httplib.InvalidURL instead of ValueError as the new error case's
19exception. (GH-13044)
20
21Backport Co-Authored-By: Miro HronĨok <miro@hroncok.cz>
22
23(cherry picked from commit 7e200e0763f5b71c199aaf98bd5588f291585619)
24
25Notes on backport to Python 2.7:
26
27* test_urllib tests urllib.urlopen() which quotes the URL and so is
28 not vulerable to HTTP Header Injection.
29* Add tests to test_urllib2 on urllib2.urlopen().
30* Reject non-ASCII characters: range 0x80-0xff.
31
32Upstream-Status: Backport
33CVE: CVE-2019-9740
34Signed-off-by: Anuj Mittal <anuj.mittal@intel.com>
35---
36 Lib/httplib.py | 16 ++++++
37 Lib/test/test_urllib.py | 25 +++++++++
38 Lib/test/test_urllib2.py | 51 ++++++++++++++++++-
39 Lib/test/test_xmlrpc.py | 8 ++-
40 .../2019-04-10-08-53-30.bpo-30458.51E-DA.rst | 1 +
41 5 files changed, 99 insertions(+), 2 deletions(-)
42 create mode 100644 Misc/NEWS.d/next/Security/2019-04-10-08-53-30.bpo-30458.51E-DA.rst
43
44diff --git a/Lib/httplib.py b/Lib/httplib.py
45index 60a8fb4e355f..1b41c346e090 100644
46--- a/Lib/httplib.py
47+++ b/Lib/httplib.py
48@@ -247,6 +247,16 @@
49 _is_legal_header_name = re.compile(r'\A[^:\s][^:\r\n]*\Z').match
50 _is_illegal_header_value = re.compile(r'\n(?![ \t])|\r(?![ \t\n])').search
51
52+# These characters are not allowed within HTTP URL paths.
53+# See https://tools.ietf.org/html/rfc3986#section-3.3 and the
54+# https://tools.ietf.org/html/rfc3986#appendix-A pchar definition.
55+# Prevents CVE-2019-9740. Includes control characters such as \r\n.
56+# Restrict non-ASCII characters above \x7f (0x80-0xff).
57+_contains_disallowed_url_pchar_re = re.compile('[\x00-\x20\x7f-\xff]')
58+# Arguably only these _should_ allowed:
59+# _is_allowed_url_pchars_re = re.compile(r"^[/!$&'()*+,;=:@%a-zA-Z0-9._~-]+$")
60+# We are more lenient for assumed real world compatibility purposes.
61+
62 # We always set the Content-Length header for these methods because some
63 # servers will otherwise respond with a 411
64 _METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'}
65@@ -927,6 +937,12 @@ def putrequest(self, method, url, skip_host=0, skip_accept_encoding=0):
66 self._method = method
67 if not url:
68 url = '/'
69+ # Prevent CVE-2019-9740.
70+ match = _contains_disallowed_url_pchar_re.search(url)
71+ if match:
72+ raise InvalidURL("URL can't contain control characters. %r "
73+ "(found at least %r)"
74+ % (url, match.group()))
75 hdr = '%s %s %s' % (method, url, self._http_vsn_str)
76
77 self._output(hdr)
78diff --git a/Lib/test/test_urllib.py b/Lib/test/test_urllib.py
79index 1ce9201c0693..d7778d4194f3 100644
80--- a/Lib/test/test_urllib.py
81+++ b/Lib/test/test_urllib.py
82@@ -257,6 +257,31 @@ def test_url_fragment(self):
83 finally:
84 self.unfakehttp()
85
86+ def test_url_with_control_char_rejected(self):
87+ for char_no in range(0, 0x21) + range(0x7f, 0x100):
88+ char = chr(char_no)
89+ schemeless_url = "//localhost:7777/test%s/" % char
90+ self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
91+ try:
92+ # urllib quotes the URL so there is no injection.
93+ resp = urllib.urlopen("http:" + schemeless_url)
94+ self.assertNotIn(char, resp.geturl())
95+ finally:
96+ self.unfakehttp()
97+
98+ def test_url_with_newline_header_injection_rejected(self):
99+ self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
100+ host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
101+ schemeless_url = "//" + host + ":8080/test/?test=a"
102+ try:
103+ # urllib quotes the URL so there is no injection.
104+ resp = urllib.urlopen("http:" + schemeless_url)
105+ self.assertNotIn(' ', resp.geturl())
106+ self.assertNotIn('\r', resp.geturl())
107+ self.assertNotIn('\n', resp.geturl())
108+ finally:
109+ self.unfakehttp()
110+
111 def test_read_bogus(self):
112 # urlopen() should raise IOError for many error codes.
113 self.fakehttp('''HTTP/1.1 401 Authentication Required
114diff --git a/Lib/test/test_urllib2.py b/Lib/test/test_urllib2.py
115index 6d24d5ddf83c..9531818e16b2 100644
116--- a/Lib/test/test_urllib2.py
117+++ b/Lib/test/test_urllib2.py
118@@ -15,6 +15,9 @@
119 except ImportError:
120 ssl = None
121
122+from test.test_urllib import FakeHTTPMixin
123+
124+
125 # XXX
126 # Request
127 # CacheFTPHandler (hard to write)
128@@ -1262,7 +1265,7 @@ def _test_basic_auth(self, opener, auth_handler, auth_header,
129 self.assertEqual(len(http_handler.requests), 1)
130 self.assertFalse(http_handler.requests[0].has_header(auth_header))
131
132-class MiscTests(unittest.TestCase):
133+class MiscTests(unittest.TestCase, FakeHTTPMixin):
134
135 def test_build_opener(self):
136 class MyHTTPHandler(urllib2.HTTPHandler): pass
137@@ -1317,6 +1320,52 @@ def test_unsupported_algorithm(self):
138 "Unsupported digest authentication algorithm 'invalid'"
139 )
140
141+ @unittest.skipUnless(ssl, "ssl module required")
142+ def test_url_with_control_char_rejected(self):
143+ for char_no in range(0, 0x21) + range(0x7f, 0x100):
144+ char = chr(char_no)
145+ schemeless_url = "//localhost:7777/test%s/" % char
146+ self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
147+ try:
148+ # We explicitly test urllib.request.urlopen() instead of the top
149+ # level 'def urlopen()' function defined in this... (quite ugly)
150+ # test suite. They use different url opening codepaths. Plain
151+ # urlopen uses FancyURLOpener which goes via a codepath that
152+ # calls urllib.parse.quote() on the URL which makes all of the
153+ # above attempts at injection within the url _path_ safe.
154+ escaped_char_repr = repr(char).replace('\\', r'\\')
155+ InvalidURL = httplib.InvalidURL
156+ with self.assertRaisesRegexp(
157+ InvalidURL, "contain control.*" + escaped_char_repr):
158+ urllib2.urlopen("http:" + schemeless_url)
159+ with self.assertRaisesRegexp(
160+ InvalidURL, "contain control.*" + escaped_char_repr):
161+ urllib2.urlopen("https:" + schemeless_url)
162+ finally:
163+ self.unfakehttp()
164+
165+ @unittest.skipUnless(ssl, "ssl module required")
166+ def test_url_with_newline_header_injection_rejected(self):
167+ self.fakehttp(b"HTTP/1.1 200 OK\r\n\r\nHello.")
168+ host = "localhost:7777?a=1 HTTP/1.1\r\nX-injected: header\r\nTEST: 123"
169+ schemeless_url = "//" + host + ":8080/test/?test=a"
170+ try:
171+ # We explicitly test urllib2.urlopen() instead of the top
172+ # level 'def urlopen()' function defined in this... (quite ugly)
173+ # test suite. They use different url opening codepaths. Plain
174+ # urlopen uses FancyURLOpener which goes via a codepath that
175+ # calls urllib.parse.quote() on the URL which makes all of the
176+ # above attempts at injection within the url _path_ safe.
177+ InvalidURL = httplib.InvalidURL
178+ with self.assertRaisesRegexp(
179+ InvalidURL, r"contain control.*\\r.*(found at least . .)"):
180+ urllib2.urlopen("http:" + schemeless_url)
181+ with self.assertRaisesRegexp(InvalidURL, r"contain control.*\\n"):
182+ urllib2.urlopen("https:" + schemeless_url)
183+ finally:
184+ self.unfakehttp()
185+
186+
187
188 class RequestTests(unittest.TestCase):
189
190diff --git a/Lib/test/test_xmlrpc.py b/Lib/test/test_xmlrpc.py
191index 36b3be67fd6b..90ccb30716ff 100644
192--- a/Lib/test/test_xmlrpc.py
193+++ b/Lib/test/test_xmlrpc.py
194@@ -659,7 +659,13 @@ def test_dotted_attribute(self):
195 def test_partial_post(self):
196 # Check that a partial POST doesn't make the server loop: issue #14001.
197 conn = httplib.HTTPConnection(ADDR, PORT)
198- conn.request('POST', '/RPC2 HTTP/1.0\r\nContent-Length: 100\r\n\r\nbye')
199+ conn.send('POST /RPC2 HTTP/1.0\r\n'
200+ 'Content-Length: 100\r\n\r\n'
201+ 'bye HTTP/1.1\r\n'
202+ 'Host: %s:%s\r\n'
203+ 'Accept-Encoding: identity\r\n'
204+ 'Content-Length: 0\r\n\r\n'
205+ % (ADDR, PORT))
206 conn.close()
207
208 class SimpleServerEncodingTestCase(BaseServerTestCase):
209diff --git a/Misc/NEWS.d/next/Security/2019-04-10-08-53-30.bpo-30458.51E-DA.rst b/Misc/NEWS.d/next/Security/2019-04-10-08-53-30.bpo-30458.51E-DA.rst
210new file mode 100644
211index 000000000000..47cb899df1af
212--- /dev/null
213+++ b/Misc/NEWS.d/next/Security/2019-04-10-08-53-30.bpo-30458.51E-DA.rst
214@@ -0,0 +1 @@
215+Address CVE-2019-9740 by disallowing URL paths with embedded whitespace or control characters through into the underlying http client request. Such potentially malicious header injection URLs now cause an httplib.InvalidURL exception to be raised.