From 4bfd8b4b8bff155be846fac70514d3b85896b768 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 23 May 2024 11:37:12 +0300 Subject: [PATCH 1/2] [3.14] gh-119452: Fix a potential virtual memory allocation denial of service in http.server The CGI server on Windows could consume the amount of memory specified in the Content-Length header of the request even if the client does not send such much data. Now it reads the POST request body by chunks, therefore the memory consumption is proportional to the amount of sent data. --- Lib/http/server.py | 15 ++++++- Lib/test/test_httpservers.py | 39 +++++++++++++++++++ ...-05-23-11-44-41.gh-issue-119452.PRfsSv.rst | 5 +++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 Misc/NEWS.d/next/Security/2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst diff --git a/Lib/http/server.py b/Lib/http/server.py index 8bb49275e78cbd..bee8605dfef95e 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -134,6 +134,10 @@ DEFAULT_ERROR_CONTENT_TYPE = "text/html;charset=utf-8" +# Data larger than this will be read in chunks, to prevent extreme +# overallocation. +_MIN_READ_BUF_SIZE = 1 << 20 + class HTTPServer(socketserver.TCPServer): allow_reuse_address = True # Seems to make sense in testing environment @@ -1284,7 +1288,16 @@ def run_cgi(self): env = env ) if self.command.lower() == "post" and nbytes > 0: - data = self.rfile.read(nbytes) + cursize = 0 + data = self.rfile.read(min(nbytes, _MIN_READ_BUF_SIZE)) + while (len(data) < nbytes and len(data) != cursize and + select.select([self.rfile._sock], [], [], self.timeout)[0]): + cursize = len(data) + # This is a geometric increase in read size (never more + # than doubling our the current length of data per loop + # iteration). + delta = min(cursize, nbytes - cursize) + data += self.rfile.read(delta) else: data = None # throw away additional data [see bug #427345] diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py index 9539457d4d829d..7b58c5ef55b337 100644 --- a/Lib/test/test_httpservers.py +++ b/Lib/test/test_httpservers.py @@ -913,6 +913,20 @@ def test_path_without_leading_slash(self): print("") """ +cgi_file7 = """\ +#!%s +import os +import sys + +print("Content-type: text/plain") +print() + +content_length = int(os.environ["CONTENT_LENGTH"]) +body = sys.stdin.buffer.read(content_length) + +print(f"{content_length} {len(body)}") +""" + @unittest.skipIf(hasattr(os, 'geteuid') and os.geteuid() == 0, "This test can't be run reliably as root (issue #13308).") @@ -952,6 +966,8 @@ def setUp(self): self.file3_path = None self.file4_path = None self.file5_path = None + self.file6_path = None + self.file7_path = None # The shebang line should be pure ASCII: use symlink if possible. # See issue #7668. @@ -1006,6 +1022,11 @@ def setUp(self): file6.write(cgi_file6 % self.pythonexe) os.chmod(self.file6_path, 0o777) + self.file7_path = os.path.join(self.cgi_dir, 'file7.py') + with open(self.file7_path, 'w', encoding='utf-8') as file7: + file7.write(cgi_file7 % self.pythonexe) + os.chmod(self.file7_path, 0o777) + os.chdir(self.parent_dir) def tearDown(self): @@ -1028,6 +1049,8 @@ def tearDown(self): os.remove(self.file5_path) if self.file6_path: os.remove(self.file6_path) + if self.file7_path: + os.remove(self.file7_path) os.rmdir(self.cgi_child_dir) os.rmdir(self.cgi_dir) os.rmdir(self.cgi_dir_in_sub_dir) @@ -1100,6 +1123,22 @@ def test_post(self): self.assertEqual(res.read(), b'1, python, 123456' + self.linesep) + def test_large_content_length(self): + for w in range(15, 25): + size = 1 << w + body = b'X' * size + headers = {'Content-Length' : str(size)} + res = self.request('/cgi-bin/file7.py', 'POST', body, headers) + self.assertEqual(res.read(), b'%d %d' % (size, size) + self.linesep) + + def test_large_content_length_truncated(self): + with support.swap_attr(self.request_handler, 'timeout', 0.001): + for w in range(18, 65): + size = 1 << w + headers = {'Content-Length' : str(size)} + res = self.request('/cgi-bin/file1.py', 'POST', b'x', headers) + self.assertEqual(res.read(), b'Hello World' + self.linesep) + def test_invaliduri(self): res = self.request('/cgi-bin/invalid') res.read() diff --git a/Misc/NEWS.d/next/Security/2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst b/Misc/NEWS.d/next/Security/2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst new file mode 100644 index 00000000000000..98956627f2b30d --- /dev/null +++ b/Misc/NEWS.d/next/Security/2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst @@ -0,0 +1,5 @@ +Fix a potential memory denial of service in the :mod:`http.server` module. +When a malicious user is connected to the CGI server on Windows, it could cause +an arbitrary amount of memory to be allocated. +This could have led to symptoms including a :exc:`MemoryError`, swapping, out +of memory (OOM) killed processes or containers, or even system crashes. From 3d1b73381210566aabafbe85d5f8fb00ebb6a95d Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 3 Dec 2025 14:23:32 +0200 Subject: [PATCH 2/2] Simply allow read() to fail with timeout. --- Lib/http/server.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index bee8605dfef95e..ac1f57c29f06ff 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -1290,14 +1290,16 @@ def run_cgi(self): if self.command.lower() == "post" and nbytes > 0: cursize = 0 data = self.rfile.read(min(nbytes, _MIN_READ_BUF_SIZE)) - while (len(data) < nbytes and len(data) != cursize and - select.select([self.rfile._sock], [], [], self.timeout)[0]): + while len(data) < nbytes and len(data) != cursize: cursize = len(data) # This is a geometric increase in read size (never more - # than doubling our the current length of data per loop + # than doubling out the current length of data per loop # iteration). delta = min(cursize, nbytes - cursize) - data += self.rfile.read(delta) + try: + data += self.rfile.read(delta) + except TimeoutError: + break else: data = None # throw away additional data [see bug #427345]