Skip to content

Commit 4ce2790

Browse files
[3.14] gh-119451: Fix a potential denial of service in http.client (GH-119454) (#142138)
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
1 parent afce34f commit 4ce2790

File tree

3 files changed

+95
-4
lines changed

3 files changed

+95
-4
lines changed

Lib/http/client.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,11 @@
111111
_MAXLINE = 65536
112112
_MAXHEADERS = 100
113113

114+
# Data larger than this will be read in chunks, to prevent extreme
115+
# overallocation.
116+
_MIN_READ_BUF_SIZE = 1 << 20
117+
118+
114119
# Header name/value ABNF (http://tools.ietf.org/html/rfc7230#section-3.2)
115120
#
116121
# VCHAR = %x21-7E
@@ -639,10 +644,25 @@ def _safe_read(self, amt):
639644
reading. If the bytes are truly not available (due to EOF), then the
640645
IncompleteRead exception can be used to detect the problem.
641646
"""
642-
data = self.fp.read(amt)
643-
if len(data) < amt:
644-
raise IncompleteRead(data, amt-len(data))
645-
return data
647+
cursize = min(amt, _MIN_READ_BUF_SIZE)
648+
data = self.fp.read(cursize)
649+
if len(data) >= amt:
650+
return data
651+
if len(data) < cursize:
652+
raise IncompleteRead(data, amt - len(data))
653+
654+
data = io.BytesIO(data)
655+
data.seek(0, 2)
656+
while True:
657+
# This is a geometric increase in read size (never more than
658+
# doubling out the current length of data per loop iteration).
659+
delta = min(cursize, amt - cursize)
660+
data.write(self.fp.read(delta))
661+
if data.tell() >= amt:
662+
return data.getvalue()
663+
cursize += delta
664+
if data.tell() < cursize:
665+
raise IncompleteRead(data.getvalue(), amt - data.tell())
646666

647667
def _safe_readinto(self, b):
648668
"""Same as _safe_read, but for reading into a buffer."""

Lib/test/test_httplib.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1465,6 +1465,72 @@ def run_server():
14651465
thread.join()
14661466
self.assertEqual(result, b"proxied data\n")
14671467

1468+
def test_large_content_length(self):
1469+
serv = socket.create_server((HOST, 0))
1470+
self.addCleanup(serv.close)
1471+
1472+
def run_server():
1473+
[conn, address] = serv.accept()
1474+
with conn:
1475+
while conn.recv(1024):
1476+
conn.sendall(
1477+
b"HTTP/1.1 200 Ok\r\n"
1478+
b"Content-Length: %d\r\n"
1479+
b"\r\n" % size)
1480+
conn.sendall(b'A' * (size//3))
1481+
conn.sendall(b'B' * (size - size//3))
1482+
1483+
thread = threading.Thread(target=run_server)
1484+
thread.start()
1485+
self.addCleanup(thread.join, 1.0)
1486+
1487+
conn = client.HTTPConnection(*serv.getsockname())
1488+
try:
1489+
for w in range(15, 27):
1490+
size = 1 << w
1491+
conn.request("GET", "/")
1492+
with conn.getresponse() as response:
1493+
self.assertEqual(len(response.read()), size)
1494+
finally:
1495+
conn.close()
1496+
thread.join(1.0)
1497+
1498+
def test_large_content_length_truncated(self):
1499+
serv = socket.create_server((HOST, 0))
1500+
self.addCleanup(serv.close)
1501+
1502+
def run_server():
1503+
while True:
1504+
[conn, address] = serv.accept()
1505+
with conn:
1506+
conn.recv(1024)
1507+
if not size:
1508+
break
1509+
conn.sendall(
1510+
b"HTTP/1.1 200 Ok\r\n"
1511+
b"Content-Length: %d\r\n"
1512+
b"\r\n"
1513+
b"Text" % size)
1514+
1515+
thread = threading.Thread(target=run_server)
1516+
thread.start()
1517+
self.addCleanup(thread.join, 1.0)
1518+
1519+
conn = client.HTTPConnection(*serv.getsockname())
1520+
try:
1521+
for w in range(18, 65):
1522+
size = 1 << w
1523+
conn.request("GET", "/")
1524+
with conn.getresponse() as response:
1525+
self.assertRaises(client.IncompleteRead, response.read)
1526+
conn.close()
1527+
finally:
1528+
conn.close()
1529+
size = 0
1530+
conn.request("GET", "/")
1531+
conn.close()
1532+
thread.join(1.0)
1533+
14681534
def test_putrequest_override_domain_validation(self):
14691535
"""
14701536
It should be possible to override the default validation
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a potential memory denial of service in the :mod:`http.client` module.
2+
When connecting to a malicious server, it could cause
3+
an arbitrary amount of memory to be allocated.
4+
This could have led to symptoms including a :exc:`MemoryError`, swapping, out
5+
of memory (OOM) killed processes or containers, or even system crashes.

0 commit comments

Comments
 (0)