From 82dc922c7c4771ef789f5b395f54a603c693b05e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Fri, 1 Jul 2016 13:08:55 +0200 Subject: [PATCH] Disabled HTTP header injections in http.client. CVE-2016-5699 python: http protocol steam injection attack rhbz#1303699: https://bugzilla.redhat.com/show_bug.cgi?id=1303699 rhbz#1351687: https://bugzilla.redhat.com/show_bug.cgi?id=1351687 Based on an upstream change by Demian Brecht and Serhiy Storchaka - in changeset 94952:bf3e1c9b80e9 3.4 - https://hg.python.org/cpython/rev/bf3e1c9b80e9 --- lib-python/3/http/client.py | 37 +++++++++++++++++++++++++ lib-python/3/test/test_httplib.py | 57 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+) diff --git a/lib-python/3/http/client.py b/lib-python/3/http/client.py index 5466d06..1d215d8 100644 --- a/lib-python/3/http/client.py +++ b/lib-python/3/http/client.py @@ -70,6 +70,7 @@ import email.parser import email.message import io import os +import re import socket import collections from urllib.parse import urlsplit @@ -207,6 +208,34 @@ MAXAMOUNT = 1048576 # maximal line length when calling readline(). _MAXLINE = 65536 +# Header name/value ABNF (http://tools.ietf.org/html/rfc7230#section-3.2) +# +# VCHAR = %x21-7E +# obs-text = %x80-FF +# header-field = field-name ":" OWS field-value OWS +# field-name = token +# field-value = *( field-content / obs-fold ) +# field-content = field-vchar [ 1*( SP / HTAB ) field-vchar ] +# field-vchar = VCHAR / obs-text +# +# obs-fold = CRLF 1*( SP / HTAB ) +# ; obsolete line folding +# ; see Section 3.2.4 + +# token = 1*tchar +# +# tchar = "!" / "#" / "$" / "%" / "&" / "'" / "*" +# / "+" / "-" / "." / "^" / "_" / "`" / "|" / "~" +# / DIGIT / ALPHA +# ; any VCHAR, except delimiters +# +# VCHAR defined in http://tools.ietf.org/html/rfc5234#appendix-B.1 + +# the patterns for both name and value are more leniant than RFC +# definitions to allow for backwards compatibility +_is_legal_header_name = re.compile(b'^[^:\s][^:\r\n]*$').match +_is_illegal_header_value = re.compile(b'\n(?![ \t])|\r(?![ \t\n])').search + class HTTPMessage(email.message.Message): # XXX The only usage of this method is in # http.server.CGIHTTPRequestHandler. Maybe move the code there so @@ -953,12 +982,20 @@ class HTTPConnection: if hasattr(header, 'encode'): header = header.encode('ascii') + + if not _is_legal_header_name(header): + raise ValueError('Invalid header name %r' % (header,)) + values = list(values) for i, one_value in enumerate(values): if hasattr(one_value, 'encode'): values[i] = one_value.encode('latin1') elif isinstance(one_value, int): values[i] = str(one_value).encode('ascii') + + if _is_illegal_header_value(values[i]): + raise ValueError('Invalid header value %r' % (values[i],)) + value = b'\r\n\t'.join(values) header = header + b': ' + value self._output(header) diff --git a/lib-python/3/test/test_httplib.py b/lib-python/3/test/test_httplib.py index 420302c..31d3bd0 100644 --- a/lib-python/3/test/test_httplib.py +++ b/lib-python/3/test/test_httplib.py @@ -134,6 +134,33 @@ class HeaderTests(TestCase): conn.putheader('Content-length', 42) self.assertTrue(b'Content-length: 42' in conn._buffer) + conn.putheader('Foo', ' bar ') + self.assertIn(b'Foo: bar ', conn._buffer) + conn.putheader('Bar', '\tbaz\t') + self.assertIn(b'Bar: \tbaz\t', conn._buffer) + conn.putheader('Authorization', 'Bearer mytoken') + self.assertIn(b'Authorization: Bearer mytoken', conn._buffer) + conn.putheader('IterHeader', 'IterA', 'IterB') + self.assertIn(b'IterHeader: IterA\r\n\tIterB', conn._buffer) + conn.putheader('LatinHeader', b'\xFF') + self.assertIn(b'LatinHeader: \xFF', conn._buffer) + conn.putheader('Utf8Header', b'\xc3\x80') + self.assertIn(b'Utf8Header: \xc3\x80', conn._buffer) + conn.putheader('C1-Control', b'next\x85line') + self.assertIn(b'C1-Control: next\x85line', conn._buffer) + conn.putheader('Embedded-Fold-Space', 'is\r\n allowed') + self.assertIn(b'Embedded-Fold-Space: is\r\n allowed', conn._buffer) + conn.putheader('Embedded-Fold-Tab', 'is\r\n\tallowed') + self.assertIn(b'Embedded-Fold-Tab: is\r\n\tallowed', conn._buffer) + conn.putheader('Key Space', 'value') + self.assertIn(b'Key Space: value', conn._buffer) + conn.putheader('KeySpace ', 'value') + self.assertIn(b'KeySpace : value', conn._buffer) + conn.putheader(b'Nonbreak\xa0Space', 'value') + self.assertIn(b'Nonbreak\xa0Space: value', conn._buffer) + conn.putheader(b'\xa0NonbreakSpace', 'value') + self.assertIn(b'\xa0NonbreakSpace: value', conn._buffer) + def test_ipv6host_header(self): # Default host header on IPv6 transaction should wrapped by [] if # its actual IPv6 address @@ -153,6 +180,36 @@ class HeaderTests(TestCase): conn.request('GET', '/foo') self.assertTrue(sock.data.startswith(expected)) + def test_invalid_headers(self): + conn = client.HTTPConnection('example.com') + conn.sock = FakeSocket('') + conn.putrequest('GET', '/') + + # http://tools.ietf.org/html/rfc7230#section-3.2.4, whitespace is no + # longer allowed in header names + cases = ( + (b'Invalid\r\nName', b'ValidValue'), + (b'Invalid\rName', b'ValidValue'), + (b'Invalid\nName', b'ValidValue'), + (b'\r\nInvalidName', b'ValidValue'), + (b'\rInvalidName', b'ValidValue'), + (b'\nInvalidName', b'ValidValue'), + (b' InvalidName', b'ValidValue'), + (b'\tInvalidName', b'ValidValue'), + (b'Invalid:Name', b'ValidValue'), + (b':InvalidName', b'ValidValue'), + (b'ValidName', b'Invalid\r\nValue'), + (b'ValidName', b'Invalid\rValue'), + (b'ValidName', b'Invalid\nValue'), + (b'ValidName', b'InvalidValue\r\n'), + (b'ValidName', b'InvalidValue\r'), + (b'ValidName', b'InvalidValue\n'), + ) + for name, value in cases: + with self.subTest((name, value)): + with self.assertRaisesRegex(ValueError, 'Invalid header'): + conn.putheader(name, value) + class BasicTest(TestCase): def test_status_lines(self): -- 2.9.0