Repository navigation
2.7.18.16: httplib unbounded headers after 100 Continue (CVE-2021-3737) - #96
Conversation
) Backport of python/cpython bpo-44022 (pythonGH-25933, 078b146) to 2.7.18.x. HTTPResponse.begin() skips the header block of each 100 Continue response. That loop limited the length of each line (_MAXLINE) but not the number of lines, so a malicious server could keep a client reading header lines forever (an infinite loop / DoS). The skipped lines are now counted and HTTPException("got more than 100 headers") is raised past _MAXHEADERS, the same limit already applied to a final response's headers. 2.7.18.x: upstream refactored the loop into _read_headers(); 2.7 parses headers with HTTPMessage.readheaders(), so the counter is added to the existing skip loop instead. The upstream test would pass on unpatched 2.7 (EOF after the 'r' lines ends in BadStatusLine, itself an HTTPException), so the 2.7 test appends a valid 200 response and checks the message; a second test checks a normal 100 Continue is still skipped. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ezequielp-activestate
left a comment
ezequielp-activestate
left a comment
There was a problem hiding this comment.
One blocker: upstream's follow-up to this CVE isn't ported.
-
What this PR fixes. It bounds the header lines inside one
100 Continueresponse. A100 Continuecarrying 100,000 header lines is now refused withHTTPException: got more than 100 headers, where the base accepted it. That part is right, andtest_httplibpasses. -
🔴 What's missing. Upstream later found that the same DoS remains through two sibling loops, and fixed it as pythongh-150743 (41cc78a, GHSA-w4q2-g22w-6fr4). The commit message says it is a "follow-up to pythongh-88188 for CVE-2021-3737, which bounded header lines within an interim response but not these two sibling loops". Both loops are still unbounded here:
- the loop in
HTTPResponse.begin()that skips interim responses (Lib/httplib.py, thewhile Truearound_read_status()). Upstream caps it at 100 responses (_MAXINTERIMRESPONSES). - the trailer loop at the end of
_read_chunked(). Upstream caps it at the header limit.
- the loop in
-
Repro (canned responses through
HTTPResponse, on both base and this branch):- 100,000 ×
HTTP/1.1 100 Continue\r\n\r\n, then a 200: accepted. - A chunked 200 with 100,000 trailer lines: accepted.
A server that streams either one forever keeps the client reading indefinitely. A socket timeout never fires, because data keeps arriving. The 3.9.25.1 and 3.10.22.1 backports already carry
_MAXINTERIMRESPONSES, so only this line is missing it. - 100,000 ×
-
Fix: port 41cc78a into this PR (a counter on the interim-response loop and on the trailer loop,
HTTPExceptionpast 100), with its two tests.
|
Agreed, thanks. I've ported 41cc78a (pythongh-150743) into this PR as eb736f0. Change
Tests (ported from upstream; 2.7 has no chunked fixtures in test_httplib, so each test builds its own body):
The two limit tests fail on the previous head and pass now. Out of scope: the header loop in |
…tplib Backport of python/cpython pythongh-150743 (41cc78a; 3.10: 1790e58), GHSA-w4q2-g22w-6fr4, to 2.7.18.x. Review follow-up to the CVE-2021-3737 fix in this PR, which bounded the header lines inside one interim response but not these two sibling loops: - HTTPResponse.begin() skipped any number of "100 Continue" responses. It now stops after _MAXINTERIMRESPONSES (100) and raises HTTPException("got more than 100 interim responses"). - _read_chunked() discarded any number of trailer lines after the last chunk. It now raises HTTPException("got more than 100 trailers") past _MAXHEADERS. A server streaming either one forever kept the client reading indefinitely, and a socket timeout cannot catch that because data keeps arriving. 2.7.18.x: % formatting instead of f-strings, and assertRaisesRegexp. 2.7 has no chunked fixtures in test_httplib, so the tests build their own bodies. test_chunked_trailers is added to show that a few trailers are still accepted. test_too_many_interim_responses and test_chunked_too_many_trailers fail on the previous head of this branch and pass here. Co-Authored-By: Claude Opus 5.5 <[email protected]>
ezequielp-activestate
left a comment
ezequielp-activestate
left a comment
There was a problem hiding this comment.
Re-review of eb736f0: no blockers. Approving.
The previous blocker was the missing upstream follow-up pythongh-150743 (41cc78a, GHSA-w4q2-g22w-6fr4): begin() skipped any number of interim 100 Continue responses, and _read_chunked() read any number of trailer lines. The new commit ports it line for line (_MAXINTERIMRESPONSES = 100, the for ... else in begin(), and the trailer counter against _MAXHEADERS). The only differences are %-formatting and 2.7's test helpers. Upstream hasn't changed either loop since, on main or 3.12–3.14.
Results on Python 2.7 built from 2.7.18.16, with a fake socket feeding the response:
| response | base | head |
|---|---|---|
100,000 × 100 Continue, then 200 |
accepted | HTTPException: got more than 100 interim responses |
3 × 100 Continue, then 200 |
200, body read |
200, body read |
chunked body + 100,000 trailer lines, read() |
accepted | HTTPException: got more than 100 trailers |
the same, read(10) |
accepted | HTTPException: got more than 100 trailers |
| chunked body + 2 trailers | accepted | accepted |
test_httplib: 55 tests on base and 61 on head, all passing (12 skipped on both because this build has no _ssl). The four new tests pass.
Agreed on leaving the header loop in HTTPConnection._tunnel() alone. Upstream and the 3.9/3.10 lines leave it unbounded too.
Not assessed: HTTPS paths (no _ssl in this build).
Backport of python/cpython bpo-44022 (CVE-2021-3737, PSF-2022-7) to 2.7.18.x. Python 2.7 reached end of life before this fix was released, so it was never applied to 2.7.
Problem:
HTTPResponse.begin()skips the headers of each100 Continueresponse. It capped the length of each line but not how many lines it would read, so a malicious server could keep the client in that loop indefinitely.Fix: count the skipped lines and raise
HTTPException("got more than 100 headers")once there are more than_MAXHEADERS. That's the same limit already applied to a final response's headers.2.7 adaptation: upstream moved this loop into
_read_headers(). 2.7 parses headers withHTTPMessage.readheaders(), so I added the counter to the existing loop instead.Tests:
test_overflowing_header_limit_after_100: unpatched 2.7 would pass upstream's version by accident, because it raisesBadStatusLineat EOF. So this test appends a valid 200 response and checks the error message. It fails on 2.7.18.15.test_headers_after_100: a normal100 Continuewith a few headers is still skipped.test_httplib,test_urllib2,test_urllib2_localnet,test_xmlrpcandtest_httpserversall pass.Part of 2.7.18.16. It targets the
2.7.18.16integration branch.Review follow-up
begin()._read_chunked().