Skip to content

Commit 2eb2af8

Browse files
authored
gh-150751: validate http.client Content-Length and chunk-size (GH-150752)
RFC 9112 defines Content-Length as 1*DIGIT and chunk-size as 1*HEXDIG, but int() also accepts a sign, underscores, surrounding whitespace and an 0x prefix, so malformed framing values were parsed instead of rejected.
1 parent 20cb7fc commit 2eb2af8

3 files changed

Lines changed: 42 additions & 8 deletions

File tree

‎Lib/http/client.py‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,11 @@
165165
# to prevent http header injection.
166166
_contains_disallowed_method_pchar_re = re.compile('[\x00-\x1f]')
167167

168+
# RFC 9112: Content-Length = 1*DIGIT and chunk-size = 1*HEXDIG.
169+
# int() is more permissive, so we match against the grammar before calling it.
170+
_is_legal_content_length = re.compile(r'[0-9]+').fullmatch
171+
_is_legal_chunk_size = re.compile(rb'[0-9a-fA-F]+').fullmatch
172+
168173
# We always set the Content-Length header for these methods because some
169174
# servers will otherwise respond with a 411
170175
_METHODS_EXPECTING_BODY = {'PATCH', 'POST', 'PUT'}
@@ -392,14 +397,8 @@ def begin(self, *, _max_headers=None):
392397
# NOTE: RFC 2616, S4.4, #3 says we ignore this if tr_enc is "chunked"
393398
self.length = None
394399
length = self.headers.get("content-length")
395-
if length and not self.chunked:
396-
try:
397-
self.length = int(length)
398-
except ValueError:
399-
self.length = None
400-
else:
401-
if self.length < 0: # ignore nonsensical negative lengths
402-
self.length = None
400+
if length and not self.chunked and _is_legal_content_length(length):
401+
self.length = int(length)
403402
else:
404403
self.length = None
405404

@@ -566,7 +565,10 @@ def _read_next_chunk_size(self):
566565
i = line.find(b";")
567566
if i >= 0:
568567
line = line[:i] # strip chunk-extensions
568+
line = line.rstrip()
569569
try:
570+
if not _is_legal_chunk_size(line):
571+
raise ValueError("invalid chunk size")
570572
return int(line, 16)
571573
except ValueError:
572574
# close the connection as protocol synchronisation is

‎Lib/test/test_httplib.py‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,6 +1348,33 @@ def test_negative_content_length(self):
13481348
self.assertEqual(resp.read(), b'Hello\r\n')
13491349
self.assertTrue(resp.isclosed())
13501350

1351+
def test_malformed_content_length(self):
1352+
# RFC 9112: Content-Length = 1*DIGIT. Values that int() accepts but
1353+
# the grammar forbids must not be used to frame the body.
1354+
for value in ('+5', '5_0'):
1355+
with self.subTest(value=value):
1356+
sock = FakeSocket(
1357+
'HTTP/1.1 200 OK\r\nContent-Length: %s\r\n\r\nHello\r\n' % value)
1358+
resp = client.HTTPResponse(sock, method="GET")
1359+
resp.begin()
1360+
self.assertIsNone(resp.length)
1361+
self.assertEqual(resp.read(), b'Hello\r\n')
1362+
resp.close()
1363+
1364+
def test_malformed_chunk_size(self):
1365+
# RFC 9112: chunk-size = 1*HEXDIG. Reject sizes that int(_, 16) accepts
1366+
# but the grammar forbids (a sign, an "0x" prefix, underscores or
1367+
# leading whitespace).
1368+
start = 'HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n'
1369+
for size in ('-5', '+5', '0x5', '1_f', ' 5'):
1370+
with self.subTest(size=size):
1371+
sock = FakeSocket(start + '%s\r\nHELLO\r\n0\r\n\r\n' % size)
1372+
resp = client.HTTPResponse(sock, method="GET")
1373+
resp.begin()
1374+
self.assertRaises(client.IncompleteRead, resp.read)
1375+
self.assertTrue(resp.isclosed())
1376+
resp.close()
1377+
13511378
def test_incomplete_read(self):
13521379
sock = FakeSocket('HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHello\r\n')
13531380
resp = client.HTTPResponse(sock, method="GET")
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
:mod:`http.client` now validates the ``Content-Length`` header and the
2+
chunked ``chunk-size`` against the RFC 9112 grammar (``1*DIGIT`` and
3+
``1*HEXDIG``) before parsing them, rejecting values such as ``+5``, ``5_0``
4+
or a ``0x``-prefixed or negative chunk size that :func:`int` would otherwise
5+
accept. This avoids framing a response differently from a strict peer.

0 commit comments

Comments
 (0)