[3.14] gh-119452: Fix a potential virtual memory allocation denial of service in...

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,
so that the memory consumption is proportional to the amount of sent
data.

diff --git a/Lib/http/server.py b/Lib/http/server.py
index 8bb49275e78cbd3c1207780af216f9b3f217e737..226ca3b16ccbeba92b9587cbbc0d6bd922f48bd7 100644 (file)
--- a/Lib/http/server.py
+++ b/Lib/http/server.py
@@ -134,6 +134,10 @@ DEFAULT_ERROR_MESSAGE = """\
 
 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 @@ class CGIHTTPRequestHandler(SimpleHTTPRequestHandler):
                                  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], [], [], 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:
                 data = None
             # throw away additional data [see bug #427345]

diff --git a/Lib/test/test_httpservers.py b/Lib/test/test_httpservers.py
index 9539457d4d829da96783a60f65516c857a6141ab..0f003064f3109c9f978b9e0779be89a55340cc99 100644 (file)
--- a/Lib/test/test_httpservers.py
+++ b/Lib/test/test_httpservers.py
@@ -913,6 +913,20 @@ for k, v in os.environ.items():
 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 @@ class CGIHTTPServerTestCase(BaseTestCase):
         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 @@ class CGIHTTPServerTestCase(BaseTestCase):
             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 @@ class CGIHTTPServerTestCase(BaseTestCase):
                 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 @@ class CGIHTTPServerTestCase(BaseTestCase):
 
         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 (file)
index 0000000..9895662
--- /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.