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

[serhiy-storchaka]

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

• Issue: OOM a potential denial of service in the CGI server on Windows #119452

[gpshead]

I've marked this Draft for now as discussion on this on the security response team list is not complete. (we'll summarize that in a public issue once it has settled)

[encukou]

See #119514 (comment) for results of the PSRT discussion.

[gpshead]

a comment that this is a geometric increase in read size (never more than doubling our current the length of data per loop iteration) could be good. otherwise it takes some staring at the code to understand what it is doing with the delta.

not a big deal though given CGIHTTPRequestHandler is gone in 3.15 onwards. :)

[miss-islington-app]

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11, 3.12, 3.13.
🐍🍒⛏🤖

[bedevere-app]

GH-142130 is a backport of this pull request to the 3.13 branch.

[bedevere-app]

GH-142131 is a backport of this pull request to the 3.12 branch.

[bedevere-app]

GH-142132 is a backport of this pull request to the 3.11 branch.

[bedevere-app]

GH-142133 is a backport of this pull request to the 3.10 branch.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 MacOS M1 Refleaks NoGIL 3.13 (tier-2) has failed when building commit 6c922bb.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1396/builds/1823) and take a look at the build logs.
4. Check if the failure is related to this commit (6c922bb) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1396/builds/1823

Failed tests:

• test.test_multiprocessing_forkserver.test_processes

Failed subtests:

• test_repr_rlock - test.test_multiprocessing_forkserver.test_processes.WithProcessesTestLock.test_repr_rlock
• test_remote_shutdown_receives_trailing_data_on_slow_socket - test.test_asyncio.test_ssl.TestSSL.test_remote_shutdown_receives_trailing_data_on_slow_socket

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.13.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/_test_multiprocessing.py", line 1493, in test_repr_rlock
    self.assertEqual('<RLock(SomeOtherThread, nonzero)>', repr(lock))
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: '<RLock(SomeOtherThread, nonzero)>' != '<RLock(None, 0)>'
- <RLock(SomeOtherThread, nonzero)>
+ <RLock(None, 0)>


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.13.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/test_asyncio/test_ssl.py", line 1595, in test_remote_shutdown_receives_trailing_data_on_slow_socket
    self.loop.run_until_complete(client(srv.addr))
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.13.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/asyncio/base_events.py", line 725, in run_until_complete
    return future.result()
           ~~~~~~~~~~~~~^^
  File "/Users/buildbot/buildarea/3.13.itamaro-macos-arm64-aws.macos-with-brew.refleak.nogil/build/Lib/test/test_asyncio/test_ssl.py", line 1573, in client
    wraps=socket_transport.write,
          ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'write'

[colesbury]

The 3.14 buildbot failure looks related to this change, but I don't see it reported on this PR: https://buildbot.python.org/#/builders/1684/builds/326

======================================================================
FAIL: test_large_content_length (test.test_httpservers.CGIHTTPServerTestCase.test_large_content_length)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "b:\uildarea\3.14.ware-win11.nondebug\build\Lib\test\test_httpservers.py", line 1132, in test_large_content_length
    self.assertEqual(res.read(), b'%d %d' % (size, size) + self.linesep)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: b'4194304 2226830\r\n' != b'4194304 4194304\r\n'

----------------------------------------------------------------------
Ran 1 test in 0.336s

[gpshead]

the timeout=0 fourth arg here makes this non-blocking. the loop won't wait for further data to arrive. so we wind up not processing all of the data.

(Claude Code Opus 4.5 looking at the buildbot failure, issue, pr & branch triaged this FWIW)

[gpshead]

why select at all here given we just want to block on reading another chunk? if the socket closes or errors, that'd return anyways.

[hugovk]

Quick PR to make it blocking: #142176

[encukou]

What should this do if the data doesn't arrive?
Wait forever, as in did before the fix?
Or doses this PR also meant fix the DoS? (I don't see how, is there a timeout setting or an implicit one?)

[serhiy-storchaka]

It was so long ago that I don't remember my train of thought, but playing around with the code I think I've recreated it.

I tested this code on Linux with have_fork = False. The tests failed because the underlying file object is unbuffered and read(n) can return less than n bytes. So the loop differs from loops in other fixes -- it continues until it reads 0 bytes. select() is needed to avoid blocking. Until now all worked on Linux and Windows on my computer and in CI tests. Perhaps this depends on the system load or on non-debug build (I only tested with the debug build).

Perhaps we need to use non-zero timeout (self.timeout?) and set it to non-None for tests.

[chris-eibl]

xmlrpc/server.py has a similar code path

cpython/Lib/xmlrpc/server.py

Lines 483 to 493 in d3c888b

	max_chunk_size = 10*1024*1024
	size_remaining = int(self.headers["content-length"])
	L = []
	while size_remaining:
	chunk_size = min(size_remaining, max_chunk_size)
	chunk = self.rfile.read(chunk_size)
	if not chunk:
	break
	L.append(chunk)
	size_remaining -= len(L[-1])
	data = b''.join(L)

but without a select. The last change there was ec1712a in 2012 adding the break if no data was read. So without a timeout it will suffer the same problem if content-length lies and less data is sent?

OTOH, does anybody out there operate a server without a timeout?

[gpshead]

Wait forever, as in did before the fix?

yes. this is intentionally a blocking read until sufficient data arrives or the connection dies. :)

fwiw, i missed this myself in the first code review. cursed unnamed parameters and presumptions about what 0 means. 😁

[gpshead]

we do not need to add timeout logic here. just remove the select. this path was always intended to block until it got enough data or the socket is closed. the issue being protected against has nothing to do with blocking by holding a connection open, it's just about memory allocation.

This 1990s era server code was never meant to provide a robust server. There's a reason we got rid of it in 3.15.

[serhiy-storchaka]

We need timeout logic here to be able to test this change.

[bedevere-app]

GH-142184 is a backport of this pull request to the 3.14 branch.