Skip to content

Commit 078b146

Browse files
bpo-44022: Fix http client infinite line reading (DoS) after a HTTP 100 Continue (GH-25916) (GH-25934)
Fixes http.client potential denial of service where it could get stuck reading lines from a malicious server after a 100 Continue response. Co-authored-by: Gregory P. Smith <[email protected]> (cherry picked from commit 47895e3) Co-authored-by: Gen Xu <[email protected]>
1 parent f4dac7e commit 078b146

File tree

3 files changed

+32
-18
lines changed

3 files changed

+32
-18
lines changed

Lib/http/client.py

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,11 @@ def getallmatchingheaders(self, name):
204204
lst.append(line)
205205
return lst
206206

207-
def parse_headers(fp, _class=HTTPMessage):
208-
"""Parses only RFC2822 headers from a file pointer.
209-
210-
email Parser wants to see strings rather than bytes.
211-
But a TextIOWrapper around self.rfile would buffer too many bytes
212-
from the stream, bytes which we later need to read as bytes.
213-
So we read the correct bytes here, as bytes, for email Parser
214-
to parse.
207+
def _read_headers(fp):
208+
"""Reads potential header lines into a list from a file pointer.
215209
210+
Length of line is limited by _MAXLINE, and number of
211+
headers is limited by _MAXHEADERS.
216212
"""
217213
headers = []
218214
while True:
@@ -224,6 +220,19 @@ def parse_headers(fp, _class=HTTPMessage):
224220
raise HTTPException("got more than %d headers" % _MAXHEADERS)
225221
if line in (b'\r\n', b'\n', b''):
226222
break
223+
return headers
224+
225+
def parse_headers(fp, _class=HTTPMessage):
226+
"""Parses only RFC2822 headers from a file pointer.
227+
228+
email Parser wants to see strings rather than bytes.
229+
But a TextIOWrapper around self.rfile would buffer too many bytes
230+
from the stream, bytes which we later need to read as bytes.
231+
So we read the correct bytes here, as bytes, for email Parser
232+
to parse.
233+
234+
"""
235+
headers = _read_headers(fp)
227236
hstring = b''.join(headers).decode('iso-8859-1')
228237
return email.parser.Parser(_class=_class).parsestr(hstring)
229238

@@ -311,15 +320,10 @@ def begin(self):
311320
if status != CONTINUE:
312321
break
313322
# skip the header from the 100 response
314-
while True:
315-
skip = self.fp.readline(_MAXLINE + 1)
316-
if len(skip) > _MAXLINE:
317-
raise LineTooLong("header line")
318-
skip = skip.strip()
319-
if not skip:
320-
break
321-
if self.debuglevel > 0:
322-
print("header:", skip)
323+
skipped_headers = _read_headers(self.fp)
324+
if self.debuglevel > 0:
325+
print("headers:", skipped_headers)
326+
del skipped_headers
323327

324328
self.code = self.status = status
325329
self.reason = reason.strip()

Lib/test/test_httplib.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -998,6 +998,14 @@ def test_overflowing_header_line(self):
998998
resp = client.HTTPResponse(FakeSocket(body))
999999
self.assertRaises(client.LineTooLong, resp.begin)
10001000

1001+
def test_overflowing_header_limit_after_100(self):
1002+
body = (
1003+
'HTTP/1.1 100 OK\r\n'
1004+
'r\n' * 32768
1005+
)
1006+
resp = client.HTTPResponse(FakeSocket(body))
1007+
self.assertRaises(client.HTTPException, resp.begin)
1008+
10011009
def test_overflowing_chunked_line(self):
10021010
body = (
10031011
'HTTP/1.1 200 OK\r\n'
@@ -1402,7 +1410,7 @@ def readline(self, limit):
14021410
class OfflineTest(TestCase):
14031411
def test_all(self):
14041412
# Documented objects defined in the module should be in __all__
1405-
expected = {"responses"} # White-list documented dict() object
1413+
expected = {"responses"} # Allowlist documented dict() object
14061414
# HTTPMessage, parse_headers(), and the HTTP status code constants are
14071415
# intentionally omitted for simplicity
14081416
blacklist = {"HTTPMessage", "parse_headers"}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
mod:`http.client` now avoids infinitely reading potential HTTP headers after a
2+
``100 Continue`` status response from the server.

0 commit comments

Comments
 (0)