[3.14] gh-119452: Read/write CGI data using worker threads

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:

Lib/test/test_httpservers.py

@@ -913,6 +913,20 @@ def test_path_without_leading_slash(self):
 print("</pre>")
 """
 
+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()

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.