diff --git a/Lib/http/server.py b/Lib/http/server.py index 8bb49275e78cbd..981f8f95062443 100644 --- a/Lib/http/server.py +++ b/Lib/http/server.py @@ -104,6 +104,7 @@ import socket import socketserver import sys +import threading import time import urllib.parse @@ -134,6 +135,10 @@ DEFAULT_ERROR_CONTENT_TYPE = "text/html;charset=utf-8" +# Data larger than this will be read in chunks, to prevent extreme +# overallocation. +_READ_BUF_SIZE = 1 << 20 + class HTTPServer(socketserver.TCPServer): allow_reuse_address = True # Seems to make sense in testing environment @@ -1283,21 +1288,62 @@ def run_cgi(self): stderr=subprocess.PIPE, env = env ) + def finish_request(): + # throw away additional data [see bug #427345, gh-34546] + while select.select([self.rfile._sock], [], [], 0)[0]: + if not self.rfile._sock.recv(1): + break + try: + p.stdin.close() + except OSError: + # already closed? + pass if self.command.lower() == "post" and nbytes > 0: - data = self.rfile.read(nbytes) + def _in_task(): + """Pipe the input into the process stdin""" + bytes_left = nbytes + # We need to wait until either there's new data in rfile, + # or the process has exited. + # This spins (with short sleeps) polling for process exit. + TIMEOUT = 0.1 + while ( + bytes_left + and not p.returncode + and select.select([self.rfile._sock], [], [], TIMEOUT)[0] + ): + data = self.rfile.read(min(bytes_left, _READ_BUF_SIZE)) + if not data: + break + bytes_left -= len(data) + p.stdin.write(data) + finish_request() + request_relay_thread = threading.Thread(target=_in_task) + request_relay_thread.start() else: data = None - # throw away additional data [see bug #427345] - while select.select([self.rfile._sock], [], [], 0)[0]: - if not self.rfile._sock.recv(1): - break - stdout, stderr = p.communicate(data) - self.wfile.write(stdout) - if stderr: - self.log_error('%s', stderr) - p.stderr.close() + finish_request() + request_relay_thread = None + def _out_task(): + """Pipe the process's stdout into the socket""" + while data := p.stdout.read(_READ_BUF_SIZE): + self.wfile.write(data) + response_relay_thread = threading.Thread(target=_out_task) + response_relay_thread.start() + stderr_chunks = [] + def _err_task(): + """Collect all of stderr, to log as single message""" + while data := p.stderr.read(_READ_BUF_SIZE): + stderr_chunks.append(data) + error_log_thread = threading.Thread(target=_err_task) + error_log_thread.start() + status = p.wait() + response_relay_thread.join() p.stdout.close() - status = p.returncode + error_log_thread.join() + self.log_error('%s', b''.join(stderr_chunks)) + p.stderr.close() + if request_relay_thread: + request_relay_thread.join() if status: self.log_error("CGI script exit status %#x", status) else: 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..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.