Skip to content

gh-119452: Fix OOM vulnerability in http.server #119455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion Lib/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,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 = 1 # Seems to make sense in testing environment
Expand Down Expand Up @@ -1224,7 +1228,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]
Expand Down
38 changes: 38 additions & 0 deletions Lib/test/test_httpservers.py
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,20 @@ def test_html_escape_filename(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).")
Expand Down Expand Up @@ -734,6 +748,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.
Expand Down Expand Up @@ -788,6 +804,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):
Expand All @@ -810,6 +831,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)
Expand Down Expand Up @@ -882,6 +905,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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
Loading