Skip to content

Commit 9e0d321

Browse files
sethmlarsonillia-v
authored andcommitted
gh-146211: Reject CR/LF in HTTP tunnel request headers (GH-146212)
(cherry picked from commit 05ed7ce) Co-authored-by: Seth Larson <seth@python.org> Co-authored-by: Illia Volochii <illia.volochii@gmail.com>
1 parent 20b4393 commit 9e0d321

File tree

3 files changed

+57
-1
lines changed

3 files changed

+57
-1
lines changed

Lib/http/client.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -972,13 +972,22 @@ def _wrap_ipv6(self, ip):
972972
return ip
973973

974974
def _tunnel(self):
975+
if _contains_disallowed_url_pchar_re.search(self._tunnel_host):
976+
raise ValueError('Tunnel host can\'t contain control characters %r'
977+
% (self._tunnel_host,))
975978
connect = b"CONNECT %s:%d %s\r\n" % (
976979
self._wrap_ipv6(self._tunnel_host.encode("idna")),
977980
self._tunnel_port,
978981
self._http_vsn_str.encode("ascii"))
979982
headers = [connect]
980983
for header, value in self._tunnel_headers.items():
981-
headers.append(f"{header}: {value}\r\n".encode("latin-1"))
984+
header_bytes = header.encode("latin-1")
985+
value_bytes = value.encode("latin-1")
986+
if not _is_legal_header_name(header_bytes):
987+
raise ValueError('Invalid header name %r' % (header_bytes,))
988+
if _is_illegal_header_value(value_bytes):
989+
raise ValueError('Invalid header value %r' % (value_bytes,))
990+
headers.append(b"%s: %s\r\n" % (header_bytes, value_bytes))
982991
headers.append(b"\r\n")
983992
# Making a single send() call instead of one per line encourages
984993
# the host OS to use a more optimal packet size instead of

Lib/test/test_httplib.py

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,51 @@ def test_invalid_headers(self):
369369
with self.assertRaisesRegex(ValueError, 'Invalid header'):
370370
conn.putheader(name, value)
371371

372+
def test_invalid_tunnel_headers(self):
373+
cases = (
374+
('Invalid\r\nName', 'ValidValue'),
375+
('Invalid\rName', 'ValidValue'),
376+
('Invalid\nName', 'ValidValue'),
377+
('\r\nInvalidName', 'ValidValue'),
378+
('\rInvalidName', 'ValidValue'),
379+
('\nInvalidName', 'ValidValue'),
380+
(' InvalidName', 'ValidValue'),
381+
('\tInvalidName', 'ValidValue'),
382+
('Invalid:Name', 'ValidValue'),
383+
(':InvalidName', 'ValidValue'),
384+
('ValidName', 'Invalid\r\nValue'),
385+
('ValidName', 'Invalid\rValue'),
386+
('ValidName', 'Invalid\nValue'),
387+
('ValidName', 'InvalidValue\r\n'),
388+
('ValidName', 'InvalidValue\r'),
389+
('ValidName', 'InvalidValue\n'),
390+
)
391+
for name, value in cases:
392+
with self.subTest((name, value)):
393+
conn = client.HTTPConnection('example.com')
394+
conn.set_tunnel('tunnel', headers={
395+
name: value
396+
})
397+
conn.sock = FakeSocket('')
398+
with self.assertRaisesRegex(ValueError, 'Invalid header'):
399+
conn._tunnel() # Called in .connect()
400+
401+
def test_invalid_tunnel_host(self):
402+
cases = (
403+
'invalid\r.host',
404+
'\ninvalid.host',
405+
'invalid.host\r\n',
406+
'invalid.host\x00',
407+
'invalid host',
408+
)
409+
for tunnel_host in cases:
410+
with self.subTest(tunnel_host):
411+
conn = client.HTTPConnection('example.com')
412+
conn.set_tunnel(tunnel_host)
413+
conn.sock = FakeSocket('')
414+
with self.assertRaisesRegex(ValueError, 'Tunnel host can\'t contain control characters'):
415+
conn._tunnel() # Called in .connect()
416+
372417
def test_headers_debuglevel(self):
373418
body = (
374419
b'HTTP/1.1 200 OK\r\n'
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Reject CR/LF characters in tunnel request headers for the
2+
HTTPConnection.set_tunnel() method.

0 commit comments

Comments
 (0)