From 0f35918585d0ec51b7724c77dcf3140a9cb678d2 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Thu, 23 May 2024 11:37:12 +0300 Subject: [PATCH 1/3] gh-119452: Fix OOM vulnerability 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 momory consumption is proportional to the amont of sent data. --- Lib/http/server.py | 12 +++++- Lib/test/test_httpservers.py | 38 +++++++++++++++++++ ...-05-23-11-44-41.gh-issue-119452.PRfsSv.rst | 3 ++ 3 files changed, 52 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..87495fbc703ba0 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. +SAFE_BUF_SIZE = 1 << 20 + class HTTPServer(socketserver.TCPServer): allow_reuse_address = True # Seems to make sense in testing environment @@ -1284,7 +1288,13 @@ 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, SAFE_BUF_SIZE)) + while (len(data) < nbytes and len(data) != cursize and + select.select([self.rfile._sock], [], [], 0)[0]): + cursize = len(data) + 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..0f003064f3109c 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,21 @@ 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): + 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..1a43293ef575a8 --- /dev/null +++ b/Misc/NEWS.d/next/Security/2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst @@ -0,0 +1,3 @@ +Fix OOM vulnerability in :mod:`http.server`, when handling the POST request +in the CGI server on Windows could cause consuming an arbitrary amount of +memory. From 64620ea4a91afb624c8d6907f004f6850067ad38 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Tue, 18 Nov 2025 15:52:08 +0200 Subject: [PATCH 2/3] Update the NEWS entry. --- .../2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 index 1a43293ef575a8..b1255e81902d78 100644 --- 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 @@ -1,3 +1,6 @@ -Fix OOM vulnerability in :mod:`http.server`, when handling the POST request -in the CGI server on Windows could cause consuming an arbitrary amount of -memory. +Fix a potential 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. +In best case this could lead to a :exc:`MemoryError` or other process crash. +In worst case it could lead to swapping which would dramatically slow down the +whole system and make it less responcible. From e6c3e7bdadf2ba422ba7a289f4f8cf06b5d2cb0e Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Mon, 1 Dec 2025 14:31:18 +0200 Subject: [PATCH 3/3] Address review comments. --- Lib/http/server.py | 7 +++++-- .../2024-05-23-11-44-41.gh-issue-119452.PRfsSv.rst | 7 +++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Lib/http/server.py b/Lib/http/server.py index 87495fbc703ba0..226ca3b16ccbeb 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -136,7 +136,7 @@ # Data larger than this will be read in chunks, to prevent extreme # overallocation. -SAFE_BUF_SIZE = 1 << 20 +_MIN_READ_BUF_SIZE = 1 << 20 class HTTPServer(socketserver.TCPServer): @@ -1289,10 +1289,13 @@ def run_cgi(self): ) if self.command.lower() == "post" and nbytes > 0: cursize = 0 - data = self.rfile.read(min(nbytes, SAFE_BUF_SIZE)) + data = self.rfile.read(min(nbytes, _MIN_READ_BUF_SIZE)) while (len(data) < nbytes and len(data) != cursize and select.select([self.rfile._sock], [], [], 0)[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: 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 index b1255e81902d78..98956627f2b30d 100644 --- 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 @@ -1,6 +1,5 @@ -Fix a potential denial of service in the :mod:`http.server` module. +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. -In best case this could lead to a :exc:`MemoryError` or other process crash. -In worst case it could lead to swapping which would dramatically slow down the -whole system and make it less responcible. +This could have led to symptoms including a :exc:`MemoryError`, swapping, out +of memory (OOM) killed processes or containers, or even system crashes.