Skip to content

Commit ebc3e0e

Browse files
metsw24-maxmiss-islington
authored andcommitted
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. (cherry picked from commit 2eb2af8) Co-authored-by: metsw24-max <metsw24@gmail.com>
1 parent 6490ca6 commit ebc3e0e

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'}
@@ -383,14 +388,8 @@ def begin(self):
383388
# NOTE: RFC 2616, S4.4, #3 says we ignore this if tr_enc is "chunked"
384389
self.length = None
385390
length = self.headers.get("content-length")
386-
if length and not self.chunked:
387-
try:
388-
self.length = int(length)
389-
except ValueError:
390-
self.length = None
391-
else:
392-
if self.length < 0: # ignore nonsensical negative lengths
393-
self.length = None
391+
if length and not self.chunked and _is_legal_content_length(length):
392+
self.length = int(length)
394393
else:
395394
self.length = None
396395

@@ -557,7 +556,10 @@ def _read_next_chunk_size(self):
557556
i = line.find(b";")
558557
if i >= 0:
559558
line = line[:i] # strip chunk-extensions
559+
line = line.rstrip()
560560
try:
561+
if not _is_legal_chunk_size(line):
562+
raise ValueError("invalid chunk size")
561563
return int(line, 16)
562564
except ValueError:
563565
# 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
@@ -1273,6 +1273,33 @@ def test_negative_content_length(self):
12731273
self.assertEqual(resp.read(), b'Hello\r\n')
12741274
self.assertTrue(resp.isclosed())
12751275

1276+
def test_malformed_content_length(self):
1277+
# RFC 9112: Content-Length = 1*DIGIT. Values that int() accepts but
1278+
# the grammar forbids must not be used to frame the body.
1279+
for value in ('+5', '5_0'):
1280+
with self.subTest(value=value):
1281+
sock = FakeSocket(
1282+
'HTTP/1.1 200 OK\r\nContent-Length: %s\r\n\r\nHello\r\n' % value)
1283+
resp = client.HTTPResponse(sock, method="GET")
1284+
resp.begin()
1285+
self.assertIsNone(resp.length)
1286+
self.assertEqual(resp.read(), b'Hello\r\n')
1287+
resp.close()
1288+
1289+
def test_malformed_chunk_size(self):
1290+
# RFC 9112: chunk-size = 1*HEXDIG. Reject sizes that int(_, 16) accepts
1291+
# but the grammar forbids (a sign, an "0x" prefix, underscores or
1292+
# leading whitespace).
1293+
start = 'HTTP/1.1 200 OK\r\nTransfer-Encoding: chunked\r\n\r\n'
1294+
for size in ('-5', '+5', '0x5', '1_f', ' 5'):
1295+
with self.subTest(size=size):
1296+
sock = FakeSocket(start + '%s\r\nHELLO\r\n0\r\n\r\n' % size)
1297+
resp = client.HTTPResponse(sock, method="GET")
1298+
resp.begin()
1299+
self.assertRaises(client.IncompleteRead, resp.read)
1300+
self.assertTrue(resp.isclosed())
1301+
resp.close()
1302+
12761303
def test_incomplete_read(self):
12771304
sock = FakeSocket('HTTP/1.1 200 OK\r\nContent-Length: 10\r\n\r\nHello\r\n')
12781305
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)