diff options
author | Miss Islington (bot) <31488909+miss-islington@users.noreply.github.com> | 2021-05-06 10:10:13 -0700 |
---|---|---|
committer | Michał Górny <mgorny@gentoo.org> | 2021-06-02 09:56:40 +0200 |
commit | 96489cf28cf3c3ceba3a7b2ba05494a296dc3ca3 (patch) | |
tree | 369467f83e8b99af941da21fac07b930a69daf43 | |
parent | ssl: Hard-disable SSLv3 to avoid automagic deps (diff) | |
download | cpython-96489cf28cf3c3ceba3a7b2ba05494a296dc3ca3.tar.gz cpython-96489cf28cf3c3ceba3a7b2ba05494a296dc3ca3.tar.bz2 cpython-96489cf28cf3c3ceba3a7b2ba05494a296dc3ca3.zip |
bpo-44022: Fix http client infinite line reading (DoS) after a HTTP 100 Continue (GH-25916) (GH-25934)
Fixes http.client potential denial of service where it could get stuck reading lines from a malicious server after a 100 Continue response.
Co-authored-by: Gregory P. Smith <greg@krypto.org>
(cherry picked from commit 47895e31b6f626bc6ce47d175fe9d43c1098909d)
Co-authored-by: Gen Xu <xgbarry@gmail.com>
-rw-r--r-- | Lib/http/client.py | 38 | ||||
-rw-r--r-- | Lib/test/test_httplib.py | 10 | ||||
-rw-r--r-- | Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst | 2 |
3 files changed, 32 insertions, 18 deletions
diff --git a/Lib/http/client.py b/Lib/http/client.py index 04cd8f7d849..b756f607d34 100644 --- a/Lib/http/client.py +++ b/Lib/http/client.py @@ -204,15 +204,11 @@ class HTTPMessage(email.message.Message): lst.append(line) return lst -def parse_headers(fp, _class=HTTPMessage): - """Parses only RFC2822 headers from a file pointer. - - email Parser wants to see strings rather than bytes. - But a TextIOWrapper around self.rfile would buffer too many bytes - from the stream, bytes which we later need to read as bytes. - So we read the correct bytes here, as bytes, for email Parser - to parse. +def _read_headers(fp): + """Reads potential header lines into a list from a file pointer. + Length of line is limited by _MAXLINE, and number of + headers is limited by _MAXHEADERS. """ headers = [] while True: @@ -224,6 +220,19 @@ def parse_headers(fp, _class=HTTPMessage): raise HTTPException("got more than %d headers" % _MAXHEADERS) if line in (b'\r\n', b'\n', b''): break + return headers + +def parse_headers(fp, _class=HTTPMessage): + """Parses only RFC2822 headers from a file pointer. + + email Parser wants to see strings rather than bytes. + But a TextIOWrapper around self.rfile would buffer too many bytes + from the stream, bytes which we later need to read as bytes. + So we read the correct bytes here, as bytes, for email Parser + to parse. + + """ + headers = _read_headers(fp) hstring = b''.join(headers).decode('iso-8859-1') return email.parser.Parser(_class=_class).parsestr(hstring) @@ -311,15 +320,10 @@ class HTTPResponse(io.BufferedIOBase): if status != CONTINUE: break # skip the header from the 100 response - while True: - skip = self.fp.readline(_MAXLINE + 1) - if len(skip) > _MAXLINE: - raise LineTooLong("header line") - skip = skip.strip() - if not skip: - break - if self.debuglevel > 0: - print("header:", skip) + skipped_headers = _read_headers(self.fp) + if self.debuglevel > 0: + print("headers:", skipped_headers) + del skipped_headers self.code = self.status = status self.reason = reason.strip() diff --git a/Lib/test/test_httplib.py b/Lib/test/test_httplib.py index 3fa0691d3ad..8333aa0eeef 100644 --- a/Lib/test/test_httplib.py +++ b/Lib/test/test_httplib.py @@ -998,6 +998,14 @@ class BasicTest(TestCase): resp = client.HTTPResponse(FakeSocket(body)) self.assertRaises(client.LineTooLong, resp.begin) + def test_overflowing_header_limit_after_100(self): + body = ( + 'HTTP/1.1 100 OK\r\n' + 'r\n' * 32768 + ) + resp = client.HTTPResponse(FakeSocket(body)) + self.assertRaises(client.HTTPException, resp.begin) + def test_overflowing_chunked_line(self): body = ( 'HTTP/1.1 200 OK\r\n' @@ -1402,7 +1410,7 @@ class Readliner: class OfflineTest(TestCase): def test_all(self): # Documented objects defined in the module should be in __all__ - expected = {"responses"} # White-list documented dict() object + expected = {"responses"} # Allowlist documented dict() object # HTTPMessage, parse_headers(), and the HTTP status code constants are # intentionally omitted for simplicity blacklist = {"HTTPMessage", "parse_headers"} diff --git a/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst b/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst new file mode 100644 index 00000000000..cf6b63e3961 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2021-05-05-17-37-04.bpo-44022.bS3XJ9.rst @@ -0,0 +1,2 @@ +mod:`http.client` now avoids infinitely reading potential HTTP headers after a +``100 Continue`` status response from the server. |